Regression for multicol reftest css-multicol-1/multicol-rule-color-inherit-002.xht traced to Bug 1266621

RESOLVED FIXED in Firefox 52

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: neerja, Assigned: neerja)

Tracking

unspecified
mozilla52
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
Created attachment 8796757 [details]
multicol-rule-color-inherit-002.xht

Hi Xidorn,

I am in the process of importing new tests for multicol properties and I noticed that this test recently started failing:
css-multicol-1/multicol-rule-color-inherit-002.xht 

Here is a link to the try run where I see the failure:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=21f73ae03d47&selectedJob=28346154

Running moz regression points to Bug 1266621. I'm not sure if this is a problem or an expected outcome of your change so I'm creating this bug. Let me know in case the test I am running is no longer valid. 

I'm attaching the test file in case my tests are not checked in by the time you see this. 

Thanks,
Neerja
Flags: needinfo?(xidorn+moz)
The result is expected to change, and should match the current spec that currentcolor becomes a computed value which should be inherited as itself rather than the corresponding actual color value.

Gérard, could you update that test accordingly?
Flags: needinfo?(xidorn+moz) → needinfo?(bugzilla)

Comment 2

2 years ago
Xidorn,

The current spec ( https://drafts.csswg.org/css-multicol/#crc ) is the same as it was 5 years ago ( https://www.w3.org/TR/css3-multicol/#crc ) with regards to 'column-rule-color'.

'column-rule-color: inherit' should inherit from the parent's computed 'column-rule-color'. If the parent's 'column-rule-color' is set (as in multicol-rule-color-inherit-001), then this is the computed value which is inherited. If the parent's 'column-rule-color' is unset (as in multicol-rule-color-inherit-002), then this parent's current color is the computed value which is inherited.


- - - - -

Relevant chunk of code from 
http://test.csswg.org/suites/css-multicol-1_dev/nightly-unstable/html4/multicol-rule-color-inherit-002.htm


div#parent
  {
    color: green;
    ...
  }

div.child
  {
    color: red;
    column-rule-color: inherit;
    ...
  }

Firefox 49 reports that div#parent's computed -moz-column-rule-color is 'green', which is correct. 

What am I missing here?

For whatever it's worth, Edge, IE11 and Opera 12.16 (Presto engine) pass multicol-rule-color-inherit-002 test.
Flags: needinfo?(bugzilla)

Comment 3

2 years ago
<meta name="assert" content="This test checks that, by default, -moz-column-rule-color is the current color applying to the element." />

The current assert text is not appropriate, not best for that multicol-rule-color-inherit-002 test: I will try to improve it.

- - - - - 

By the way, if we use "-moz" vendor prefix, just like in attachment 8796757 [details] , then Firefox 49 passes the test! 

"
The test does not use proprietary features (vendor-prefixed or otherwise).
"
http://testthewebforward.org/docs/review-checklist.html

So, what's the problem?

Comment 4

2 years ago
1 other thing to double-check in attachment 8796757 [details]:

line 14: src: url("../../../fonts/Ahem.ttf")

This linkage of the font is incorrect. From Firefox's console:

"
downloadable font: download failed (font-family: "Ahem" style:normal weight:normal stretch:normal src index:0): status=2147746065 source: https://bug1306769.bmoattachments.org/fonts/Ahem.ttf
"

Comment 5

2 years ago
The linkage at line 14: src: url("../../../fonts/Ahem.ttf") in attachment 8796757 [details] is definitely wrong. When the Ahem font is correctly linked, the height of column rules become correct and will match the reference file.

Comment 6

2 years ago
> <meta name="assert" content="This test checks that, by default,
> -moz-column-rule-color is the current color applying to the element." />
> 
> The current assert text is not appropriate, not best for that
> multicol-rule-color-inherit-002 test: I will try to improve it.

{
[css-multicol-1] Improve the multicol-rule-color-inherit-002.xht
test text assert to better reflect the complexity of the test.

<meta name="assert" content="This test checks that, by default, column-rule-color is the current color applying to the element. In this test, div#parent's computed 'column-rule-color' is given by div#parent's 'currentcolor', which is 'green'. And div.child's 'column-rule-color' will take such specified value (green) from its parent (due to 'inherit' keyword) and not use its own current color." />
}

http://hg.csswg.org/test/rev/06188b6e1875

http://test.csswg.org/source/css-multicol-1/multicol-rule-color-inherit-002.xht 
test has been updated
The spec change in question is:
https://drafts.csswg.org/css-color-4/#currentcolor-color

When 'currentcolor' is inherited, what is inherited is not a color but the 'currentcolor' value.  The resulting value then takes its color from the element's color at used value time.

This is implemented in Firefox nightly but not release.
Flags: needinfo?(bugzilla)

Comment 8

2 years ago
> When 'currentcolor' is inherited, what is inherited is not a color but the
> 'currentcolor' value.  The resulting value then takes its color from the
> element's color at used value time.

Okay. I have updated the test (link, text assert, date) and I have verified that Firefox 52 passes the modified test:

http://hg.csswg.org/test/rev/4de365a89bf3

http://test.csswg.org/source/css-multicol-1/multicol-rule-color-inherit-002.xht

Updated

2 years ago
Flags: needinfo?(bugzilla)
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → INVALID
Let's not resolve this quite yet -- we still have the old (failing) version of this test in our tree, and we still have it marked as "fails" with a reference to this bug. (since this bug was filed to track the failure).

--> reopening, let's close this after we've pulled the updated version of the test & removed the "fails" annotation (as we should be able to do, now)

Neerja, perhaps you'd be able to do that? I think you should be able to remove this test from failures.list and re-run the import script. (like in bug 1295261)
Status: RESOLVED → REOPENED
Flags: needinfo?(npancholi)
Resolution: INVALID → ---
(Assignee)

Updated

2 years ago
Assignee: nobody → npancholi
Flags: needinfo?(npancholi)
(Assignee)

Updated

2 years ago
Depends on: 1314809
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment on attachment 8807701 [details]
Bug 1306769 - Update tests from CSSWG repo and remove fails annotation for css-multicol-1/multicol-rule-color-inherit-002.xht

https://reviewboard.mozilla.org/r/90780/#review90512

Looks good, thanks!
Attachment #8807701 - Flags: review?(dholbert) → review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
(Assignee)

Comment 13

2 years ago
mozreview-review-reply
Comment on attachment 8807701 [details]
Bug 1306769 - Update tests from CSSWG repo and remove fails annotation for css-multicol-1/multicol-rule-color-inherit-002.xht

https://reviewboard.mozilla.org/r/90780/#review90512

Thanks Daniel!

Comment 14

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/fd66d4e4231e
Update tests from CSSWG repo and remove fails annotation for css-multicol-1/multicol-rule-color-inherit-002.xht r=dholbert
Keywords: checkin-needed

Comment 15

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fd66d4e4231e
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.