Closed Bug 1347164 Opened 7 years ago Closed 7 years ago

Regression - Ringmark failure - hsla() color serializing as rgb() rather than rgba()

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Version:
52 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox-esr45 --- unaffected
firefox52 - wontfix
firefox-esr52 53+ fixed
firefox53 + fixed
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: mkaply, Assigned: jerry)

References

(
URL
)

Details

(Keywords: regression, site-compat)

Attachments

(9 files, 11 obsolete files)

use rgba() if the color has non-opaque value. r=dholbert
1.58 KB, patch
Details | Diff | Splinter Review
css-color computed style test. v2. r=dholbert
1.76 KB, patch
Details | Diff | Splinter Review
rewrite mochitest test_specified_value_serialization.html to look more like test_computed_style.html.
6.31 KB, patch
dholbert
: review+
Details | Diff | Splinter Review
css-color specified style test. v3
1.88 KB, patch
dholbert
: review+
Details | Diff | Splinter Review
set expected-fail for css-color-4 test with stylo.
8.83 KB, patch
xidorn
: review+
Details | Diff | Splinter Review
[aurora] serialize colors using "rgba()" as the color-function, if they have a non-opaque alpha channel.
1.58 KB, patch
gchang
: approval-mozilla-aurora+
Details | Diff | Splinter Review
[aurora] set expected-fail for css-color-4 test with stylo.
7.70 KB, patch
Details | Diff | Splinter Review
[beta] serialize colors using "rgba()" as the color-function, if they have a non-opaque alpha channel.
1.72 KB, patch
gchang
: approval-mozilla-beta+
Details | Diff | Splinter Review
[esr52] serialize colors using "rgba()" as the color-function, if they have a non-opaque alpha channel.
1.72 KB, patch
jcristau
: approval-mozilla-esr52+
Details | Diff | Splinter Review 
The following test in Ringmark is failing on Firefox 52:

var bgcolor,
elem = document.createElement("div");

// Same as rgba(), in fact, browsers re-map hsla() to rgba() internally,
//   except IE9 who retains it as hsla

elem.style.cssText = "background-color:hsla(120,40%,100%,.5)";

bgcolor = elem.style.backgroundColor;

assert( H.test.string( bgcolor, "rgba" ) || H.test.string( bgcolor, "hsla" ), "HSLA color supported" );

In Firefox 51, bgcolor is rgba(255, 255, 255, 0.5)
In Firefox 52, bgcolor is rgb(255, 255, 255, 0.5)

This code was changed in bug 1295456 and/or bug 1302787.
Seems like this should go by the principle of serializing to the more-compatible syntax, and not change the serialization to the new syntax.  Or does the spec say otherwise?  If it does, have any other browsers implemented what it says?
Flags: needinfo?(hshih)
Flags: needinfo?(dholbert)
(In reply to David Baron :dbaron: ⌚️UTC-7 from comment #1)
> Seems like this should go by the principle of serializing to the
> more-compatible syntax, and not change the serialization to the new syntax.

That's fair. So I think the tentative fix here would be:
 - When we're serializing: if the alpha is partially transparent, serialize with "rgba" instead of "rgb" (for interop/backwards-compat purposes)?

That seems reasonable to me.

(Side note: I thought I recalled discussing this in bug 1295456, but I think there we mostly focused on something orthogonal, starting with bug 1295456 comment 48 -- there, we were mostly focusing on: "if an author specifies 'rgba(..., 1.0)', do we need to bother encoding the fact that the author used rgba and had a *hardcoded* 1.0 there, or can we treat it the same as 'rgb(...)' and just disregard the 1.0 alpha component?"  For that question, I still think we made the right call.)

> Or does the spec say otherwise?  If it does, have any other browsers
> implemented what it says?

The spec does not say otherwise.  Actually, I'm now seeing that it *requires* the behavior described in my suggested fix here. Quoting it, paraphrasing/clipping quite a bit:
 # * The computed value and used value of [...] colors with an explicitly opaque alpha channel
 # [or] without an alpha channel [...]
 # is the equivalent numeric value in comma separated rgb() notation omitting the alpha value.
 #
 # * The computed value and used value of [...] colors with a non opaque alpha channel [...]
 # is the equivalent numeric value in comma separated rgba() notation with the alpha value. 
https://drafts.csswg.org/css-color-4/#resolving-color-values

There's also older spec-text at https://www.w3.org/TR/cssom-1/#serializing-css-values which says basically the same thing.

Jerry, would you be up for taking this?
Flags: needinfo?(dholbert)
(Note: the css-color-4 spec-text that I quoted in comment 2 (the "Resolving Color values" section) was only added 2 months ago, here: https://github.com/w3c/csswg-drafts/commit/193d58bd45b1901b9db5e644e525d14fe3a945be

So that's why we didn't see it when landing bug 1295456 (5 months ago).  At that time, the best hint that the spec had about this was "for legacy reasons, an rgba() function also exists, with an identical grammar and behavior to rgb().")
OK, so our behavior here is already a little subtle. Some results from the console.

Testing rgba with 1.0 alpha:
>> $0.style.color = "rgba(1, 2, 3, 1.0)"; $0.style.color
> rgb(1, 2, 3)
So, we already drop 1.0 "a" values (and collapse rgba to rgb when we do).

Testing rgba with 0.5 alpha:
>> $0.style.color = "rgba(1, 2, 3, 0.5)"; $0.style.color
> rgba(1, 2, 3, 0.5)
So, we do preserve rgba when it was specified & when there's a non-opaque alpha component (which I'd forgotten).

Testing non-rgba functions with 0.5 alpha:
>> $0.style.color = "rgb(1, 2, 3, 0.5)"; $0.style.color
> rgb(1, 2, 3, 0.5)
>> $0.style.color = "hsla(0deg, 0%, 0%, 0.5)"; $0.style.color
> rgb(0, 0, 0, 0.5)

This last chunk of output is the problematic stuff, with respect to this bug.  This comes from the logic here:
https://dxr.mozilla.org/mozilla-central/rev/6d38ad302429c98115c354d643e81987ecec5d3c/layout/style/nsCSSValue.cpp#1668-1673

>       bool showAlpha = (a != 255);
> 
>       if (unit == eCSSUnit_RGBAColor && showAlpha) {
>         aResult.AppendLiteral("rgba(");
>       } else {
>         aResult.AppendLiteral("rgb(");
>       }

I think we should just drop the "unit" check there -- that'll probably bring us into alignment with the spec text that I quoted in comment 2.  We may need a similar change in nsComputedDOMStyle, too -- not sure. And we should include tests (if we don't already have them) for this serialization behavior.
Right.  So this is a regression from:
https://hg.mozilla.org/mozilla-central/rev/a29d5cb24af486be4c8d66cdc2a024d6badf787b
where the nsCSSValue.cpp change didn't consider hsla().
No longer blocks: 1302787
status-firefox52: --- → affected
status-firefox53: --- → affected
status-firefox54: --- → affected
status-firefox55: --- → affected
status-firefox-esr45: --- → unaffected
status-firefox-esr52: --- → affected
Summary: Regression - Ringmark failure - HSLA color not translating properly → Regression - Ringmark failure - hsla() color serializing as rgb() rather than rgba()
I will try to fix this today.
Assignee: nobody → hshih
Flags: needinfo?(hshih)
MozReview-Commit-ID: 44T8gy7UWFJ
Attachment #8847505 - Flags: review?(dholbert)
Attached patch css-color computed style test. (obsolete) — — Splinter Review 
MozReview-Commit-ID: 36qT5LxhB9Z
Attachment #8847506 - Flags: review?(dholbert)
Attached patch css-color specified style test. (obsolete) — — Splinter Review 
MozReview-Commit-ID: DQln6jtxolg
Attachment #8847508 - Flags: review?(dholbert)
Comment on attachment 8847505 [details] [diff] [review]
only omit the alpha component for full opaque color value.

Review of attachment 8847505 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nits addressed.

First, the commit message needs some changes:
> Bug 1347164 - only omit the alpha component for full opaque color value. r=dholbert

This doesn't accurately describe the code-change here.  (The code about omitting the alpha component is *not* actually changing at all.  The only thing changing here is our selection of which color-function *name* to use -- rgb vs rgba.)

Please reword to clarify what's actually changing -- maybe something like:
Bug 1347164 - serialize colors using "rgba()" as the color-function, if they have a nonopaque alpha channel. r=dholbert

::: layout/style/nsCSSValue.cpp
@@ +1680,5 @@
>          unit == eCSSUnit_RGBAColor) {
>        nscolor color = GetColorValue();
>        // For brevity, we omit the alpha component if it's equal to 255 (full
> +      // opaque).
> +      // e.g.

This only partially explains what we're doing here.

Please add a note about rgb vs rgba. Perhaps expand this part of the comment to something like the following:
      // For brevity, we omit the alpha component if it's equal to 255 (full
      // opaque). Also, we use "rgba" rather than "rgb" when we do include the
      // alpha value, for backwards-compat (even though they're aliases as of
      // css-color-4). e.g.:
Attachment #8847505 - Flags: review?(dholbert) → review+
Comment on attachment 8847506 [details] [diff] [review]
css-color computed style test.

Review of attachment 8847506 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with a comment added:

::: layout/style/test/test_computed_style.html
@@ +376,5 @@
>  })();
>  
> +(function test_bug_1347164() {
> +  var color = [
> +    ["rgba(0, 0, 0, 1)", "rgb(0, 0, 0)"],

Please add a comment at the top of this to explain what you're testing in general here. I think this would do it:

 // Test that computed color values are serialized as "rgb()"
 // IFF they're fully-opaque (and otherwise as "rgba()").
Attachment #8847506 - Flags: review?(dholbert) → review+
Comment on attachment 8847508 [details] [diff] [review]
css-color specified style test.

Review of attachment 8847508 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/style/test/test_specified_style.html
@@ +1,4 @@
> +<!DOCTYPE HTML>
> +<html>
> +<head>
> +  <title>Test for miscellaneous specified style issues</title>

r=me with one followup and a comment added.

Followup request:  So, I suppose this new mochitest makes sense as an analog of the existing "test_computed_style.html" mochitest.  HOWEVER, we do *already* have a mochitest called "test_specified_value_serialization.html" which seems like it's trying to do the same thing (test a variety of serialization issues). I suspect the setup in this new test is probably better (it seems more modular, with each part nicely-scoped & cleaning up after itself), so this is fine for now, but could you file a followup on merging test_specified_value_serialization.html into this test, and address it after this bug?  I don't want to leave this in a state where we've got two "grab bag" tests that purport to be covering the same thing.

@@ +19,5 @@
> +var frame_container = document.getElementById("display");
> +
> +(function test_bug_1347164() {
> +  var color = [
> +    ["rgba(0, 0, 0, 1)", "rgb(0, 0, 0)"],

As with the previous patch, please add a comment here explaining what you're trying to test.  (probably using similar language to the suggested text in Comment 11)
Attachment #8847508 - Flags: review?(dholbert) → review+
Comment on attachment 8847508 [details] [diff] [review]
css-color specified style test.

Could these tests be added to test_specified_value_serialization.html instead of adding a new test file?  (test_specified_value_serialization is a better name for this test than test_specified_style, and there's already a test with that name that does similar things!)
Nominating for tracking, both since it sounds like ringmark matters to some partners, and since this was a regression that we shouldn't have shipped...
tracking-firefox52: --- → ?
tracking-firefox53: --- → ?
Oops, my comment 13 duplicates dholbert's comment 12 -- but I think it's simple enough to just do that here rather than a followup bug.  Just stick the existing body of the test in a (function() {...})();, and then add your new thing after it.
Yeah, good point that the (existing) test_specified_value_serialization is better-named -- we should just extend (and maybe eventually improve the older bits of) that test, rather than creating a new test file and reshuffling things into there.

essentially, +1 to comment 15.
Track 53+ as regression.
tracking-firefox53: ? → +
Patrick Kettner (a PM for Edge at Microsoft) mentioned running into this bug, too -- he says "modernizr's hsla detect breaks because of it": https://twitter.com/patrickkettner/status/842475556113338369
So, sites that use modernizr & that care about hsla support might've been broken (or at least changed their behavior) due to this bug.

This is more support for getting this uplifted to Firefox 53 before it ships.
Should we be creating web-platform-tests for this then rather than adding mochitests?
(In reply to Ryan VanderMeulen [:RyanVM] from comment #19)
> Should we be creating web-platform-tests for this then rather than adding
> mochitests?

This won't actually be adding mochitests (once comment 16 is addressed) -- it'll just be extending already-existing mochitests.

But ideally yes, it'd be great to get web-platform-tests for this behavior.  I don't think we want to gate this fix shipping on those new tests being written, though, given that we're wanting to uplift it all the way to beta & hence time is of the essence (so we can detect & address fallout sooner rather than later)
MozReview-Commit-ID: 44T8gy7UWFJ
Attachment #8847505 - Attachment is obsolete: true
Attachment #8847506 - Attachment is obsolete: true
Attached patch css-color specified style test. v2 (obsolete) — — Splinter Review 
move the test into test_specified_value_serialization.html
Attachment #8848263 - Flags: review?(dholbert)
Attachment #8847508 - Attachment is obsolete: true
Attachment #8848261 - Attachment description: update for comment 10. → use rgba() if the color has non-opaque value.
Attachment #8848262 - Attachment description: update for comment 11. → css-color computed style test. v2. r=dholbert
Attachment #8848261 - Attachment description: use rgba() if the color has non-opaque value. → use rgba() if the color has non-opaque value. r=dholbert
Attachment #8848263 - Attachment description: update for comment 12 and 13. → css-color specified style test. v2
Comment on attachment 8848263 [details] [diff] [review]
css-color specified style test. v2

Review of attachment 8848263 [details] [diff] [review]:
-----------------------------------------------------------------

This patch is simultaneously rewriting/reindenting most of this patch, and also adding new test code to it. Could you split it up into two parts, with the new test code for this bug in the second part?

(The first part should have a commit message like "Rewrite mochitest test_specified_value_serialization.html to look more like test_computed_style.html", since I think that's what you're doing.)

::: layout/style/test/test_specified_value_serialization.html
@@ +8,5 @@
> +<body>
> +<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=">Mozilla Bug </a>
> +<p id="display"></p>
> +<div id="content" style="display: none">
> +  

end-of-line whitespace
Attachment #8848263 - Flags: review?(dholbert) → review-
> This patch is simultaneously rewriting/reindenting most of this patch

Sorry, I meant to say "...of this test"
Attachment #8848263 - Attachment is obsolete: true

Attachment #8848271 -
Flags: review?(dholbert)



The hsla test in http://rng.io/about/ is passed now.
Status: NEW → ASSIGNED



Comment on attachment 8848270 [details] [diff] [review]
rewrite mochitest test_specified_value_serialization.html to look more like test_computed_style.html.

Review of attachment 8848270 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks. r=me

Attachment #8848270 -
Flags: review?(dholbert) → review+



Comment on attachment 8848271 [details] [diff] [review]
css-color specified style test. v3

Review of attachment 8848271 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

Attachment #8848271 -
Flags: review?(dholbert) → review+



Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/36a2230fbad6
Serialize colors using "rgba()" as the color-function, if they have a non-opaque alpha channel. r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/8af7daaf5f88
css-color computed style test. r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/18c70bc07c6d
Rewrite mochitest test_specified_value_serialization.html to look more like test_computed_style.html. r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb325d01c421
css-color specified style test. r=dholbert
Keywords: checkin-needed



had to back this out for turning stylo tests perma orange on inbound like https://treeherder.mozilla.org/logviewer.html#?job_id=84513380&repo=mozilla-inbound so seems to be a failure in own test.
Flags: needinfo?(hshih)









Attached patch


set fail-if for css-color-4 test with stylo. (obsolete)
— 
— Splinter Review





Currently, stylo doesn't have css-color-4 implementation. Set fail-if for these tests.

Attachment #8848426 -
Flags: review?(cam)



Comment on attachment 8848426 [details] [diff] [review]
set fail-if for css-color-4 test with stylo.

Review of attachment 8848426 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, instead of disabling the entire tests in stylo, we should update layout/style/test/stylo-failures.md so that it just expects the failures for these color values.  If you have trouble coming up with the right modifications to that file to capture the expected failures, please needinfo xidorn.

Attachment #8848426 -
Flags: review?(cam) → review-





Currently, stylo doesn't have css-color-4 implementation. Set expected-fail for these tests.

The try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1246bc0a704651d6a2ae8cf7ead98c09b9b6a976

Attachment #8849030 -
Flags: review?(xidorn+moz)

Attachment #8848426 -
Attachment is obsolete: true

Attachment #8849030 -
Flags: review?(xidorn+moz) → review+



Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/049b04f008d6
serialize colors using "rgba()" as the color-function, if they have a non-opaque alpha channel. r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7aff74601c8
css-color computed style test. r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/12704956b400
rewrite mochitest test_specified_value_serialization.html to look more like test_computed_style.html. r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e0f51a3a733
css-color specified style test. r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/a49749aa7711
set expected-fail for css-color-4 test with stylo. r=xidorn
Keywords: checkin-needed



Hi Daniel and Gerry,

This is marked tracking-firefox53+. Are we going to uplift to 53, 54 and esr52?
Flags: needinfo?(gchang)
Flags: needinfo?(dholbert)



(In reply to Jerry Shih[:jerry] (UTC+8) from comment #43)
> This is marked tracking-firefox53+. Are we going to uplift to 53, 54 and
> esr52?

I think we should, per comment 18 & comment 20.

To get that ball rolling, please request aurora/beta/esr52 approval at https://bugzilla.mozilla.org/attachment.cgi?id=8848261&action=edit
Flags: needinfo?(dholbert)
Flags: needinfo?(gchang) → needinfo?(hshih)





MozReview-Commit-ID: 44T8gy7UWFJ

--HG--
extra : rebase_source : 58f8621c70e0acff99f95d3305ef7ef0cc11c870









Attached patch


[aurora] css-color computed style test. (obsolete)
— 
— Splinter Review





MozReview-Commit-ID: 36qT5LxhB9Z









Attached patch


[aurora] css-color specified style test. (obsolete)
— 
— Splinter Review





MozReview-Commit-ID: 6RRhz8ftEVO





Currently, stylo doesn't have css-color-4 implementation. Set expected-fail for these tests.









Attached patch


[beta] css-color computed style test. (obsolete)
— 
— Splinter Review





MozReview-Commit-ID: 36qT5LxhB9Z









Attached patch


[beta] css-color specified style test. (obsolete)
— 
— Splinter Review





MozReview-Commit-ID: 6RRhz8ftEVO

Attachment #8849820 -
Attachment is obsolete: true

Attachment #8849821 -
Attachment is obsolete: true

Attachment #8849822 -
Attachment is obsolete: true

Attachment #8849823 -
Attachment is obsolete: true

Attachment #8849832 -
Attachment is obsolete: true

Attachment #8849833 -
Attachment is obsolete: true

Attachment #8849834 -
Attachment is obsolete: true



Comment on attachment 8849819 [details] [diff] [review]
[aurora] serialize colors using "rgba()" as the color-function, if they have a non-opaque alpha channel.

Approval Request Comment
[Feature/Bug causing the regression]:
bug 1295456

[User impact if declined]:
The hsla test will falied in http://rng.io/about/

[Is this code covered by automated tests?]:
Yes.
But the tests are only at m-c. Should we need to uplift the test case? If yes, there are 4 additional file need to be uplifted.

The try for m-c: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1246bc0a704651d6a2ae8cf7ead98c09b9b6a976

[Has the fix been verified in Nightly?]:
Yes.
The hsla test is passed in http://rng.io/about/ .

[Needs manual test from QE? If yes, steps to reproduce]: 
no.

[List of other uplifts needed for the feature/fix]:
Should we need to uplift the test case? If yes, there are 4 additional files need to be uplifted.

[Is the change risky?]:
no.

[Why is the change risky/not risky?]:
We will turn to use more-compatible syntax for css-color value serializing.

[String changes made/needed]:
none
Flags: needinfo?(hshih)

Attachment #8849819 -
Flags: approval-mozilla-aurora?



Comment on attachment 8849831 [details] [diff] [review]
[beta] serialize colors using "rgba()" as the color-function, if they have a non-opaque alpha channel.

Approval Request Comment
[Feature/Bug causing the regression]:
bug 1295456

[User impact if declined]:
The hsla test will be falied in http://rng.io/about/

[Is this code covered by automated tests?]:
Yes.
But the tests are only at m-c. Should we need to uplift the test case? If yes, there are 4 additional file need to be uplifted.

The try for m-c: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1246bc0a704651d6a2ae8cf7ead98c09b9b6a976

[Has the fix been verified in Nightly?]:
Yes.
The hsla test is passed in http://rng.io/about/ .

[Needs manual test from QE? If yes, steps to reproduce]: 
no.

[List of other uplifts needed for the feature/fix]:
Should we need to uplift the test case? If yes, there are 4 additional files need to be uplifted.

[Is the change risky?]:
no.

[Why is the change risky/not risky?]:
We will turn to use more-compatible syntax for css-color value serializing.

[String changes made/needed]:
none

Attachment #8849831 -
Flags: approval-mozilla-beta?



Comment on attachment 8849835 [details] [diff] [review]
[esr52] serialize colors using "rgba()" as the color-function, if they have a non-opaque alpha channel.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
We are try to use more-compatible syntax for css-color value serializing after bug 1295456 changes.

User impact if declined:
The hsla test will falied in http://rng.io/about/ 

Fix Landed on Version:
firefox55

Risk to taking this patch (and alternatives if risky): 
No. We will use more-compatible syntax for css-color value serializing.

String or UUID changes made by this patch: 
None.

Attachment #8849835 -
Flags: approval-mozilla-esr52?



Hi :jerry,
Please also uplift the test cases. Thanks.
Flags: needinfo?(hshih)



My assumption was that the test patches didn't require rebasing, only the main one. We'll make sure to land the tests too once approved :)

Also, is this still on the radar for dot release consideration?
Flags: in-testsuite+



(In reply to Ryan VanderMeulen [:RyanVM] from comment #59)
> Also, is this still on the radar for dot release consideration?

Given that we're still unaware of any actual site breakage from this (and given that there's a small risk to any behavior change), I think we should just let this ship in 53 (via uplift to 53 beta) - no need to rush it in a dot release.
Keywords: compat



Comment on attachment 8849819 [details] [diff] [review]
[aurora] serialize colors using "rgba()" as the color-function, if they have a non-opaque alpha channel.

Fix a regression related to site-compat. Aurora54+ & Beta53+.

Attachment #8849819 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Attachment #8849831 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+



Tomcat, see the recent commments. The tests need uplifting too.
Flags: needinfo?(cbook)



thanks ryan,

jerry uplifting the tests, but the set expected-fail for css-color-4 test with stylo test/patch need rebasing:

grafting 406330:a49749aa7711 "Bug 1347164 - set expected-fail for css-color-4 test with stylo. r=xidorn"
merging layout/style/test/stylo-failures.md
merging layout/style/test/test_computed_style.html
merging layout/style/test/test_specified_value_serialization.html
warning: conflicts while merging layout/style/test/stylo-failures.md! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(cbook)



We aren't running Stylo tests on !trunk, are we? That sounds like a bug if we are :)



(In reply to Ryan VanderMeulen [:RyanVM] from comment #67)
> We aren't running Stylo tests on !trunk, are we? That sounds like a bug if
> we are :)

yup just trunk so its not a urgent thing



Why would we need to backport that patch at all then?



Comment on attachment 8849823 [details] [diff] [review]
[aurora] set expected-fail for css-color-4 test with stylo.

Thanks, Ryan and Carsten.
Here is the rebased stylo failure-list patch for aurora if we still try to pass the stylo test at aurora. We need to disable some tests with stylo. The css-color-4 is not implemented in stylo.

And here is its try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fee38a43df204217b9266e0311e747f0949565b7

Attachment #8849823 -
Attachment is obsolete: false
Flags: needinfo?(ryanvm)
Flags: needinfo?(hshih)
Flags: needinfo?(cbook)



thanks jerry, will check this in
Flags: needinfo?(ryanvm)
Flags: needinfo?(cbook)



Comment on attachment 8849835 [details] [diff] [review]
[esr52] serialize colors using "rgba()" as the color-function, if they have a non-opaque alpha channel.

webcompat fix, esr52+

Attachment #8849835 -
Flags: approval-mozilla-esr52? → approval-mozilla-esr52+



Setting qe-verify- based on Jerry's assessment on manual testing needs and the fact that this fix has automated coverage (see Comment 56).
Flags: qe-verify-

You need to log in
before you can comment on or make changes to this bug.

















Attachment


















General



Creator: 






Created: 


Updated: 


Size: