1280312 - If outline-color: invert is not supported, it should compute to currentColor when using outline shorthand

Status: RESOLVED INVALID

(Core :: CSS Parsing and Computation, defect)

Description

[Louis Lazaris]

Attachment: invert.html

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160604131506

Steps to reproduce:

Set an element's color using the color property in the CSS. Set an element's outline-color property to "invert" using outline shorthand (the outline property).


Actual results:

The computed value of the outline-color property for the element is not currentColor. See demo: https://jsbin.com/qatepo/edit?html,css,output


Expected results:

The computed value of the outline-color property for the element should be currentColor (in this case, yellow).

Comment 1

[Gérard Talbot]

Impressive webs, you have something very interesting. Firefox passes the longhand version of your test but it fails the shorthand of your test. 

Eg. Firefox 47 buildID=20160606114208 and Firefox 50.0a1 buildID=20160614030904 pass this test:

http://test.csswg.org/suites/css2.1/nightly-unstable/html4/outline-color-174.htm

but its equivalent with shorthand form is not passed. I will create a test and submit it to W3C CSS2.1 test suite for inclusion. It will block bug 605520 also.

I need to check if there is a duplicate bug report of your bug report now ...

Comment 2

[Gérard Talbot]

Reduced test
------------
http://www.gtalbot.org/BugzillaSection/Bug1280312-outline-color-invert-outline-shorthand.html


Reference file
--------------
http://www.gtalbot.org/BugzillaSection/Bug1280312-outline-color-invert-outline-longhand.html


The only difference between the reduced test and the reference is lines 39, 40 and 41.

Comment 3

[Gérard Talbot]

Notes
-----
- Component has been set to Layout but it could be instead "CSS Parsing and Comp" since the longform works as expected
- If one day Firefox supports color inversion of pixels, then the current reference file will still be good, reliable but the pass-fail-conditions sentence will be misleading or wrong. I would need more time to modify the test to make it more versatile, offering 2 possible pass layouts.
- IE11 and Edge pass the test; Chrome 51 and Chrome 53 under Linux fail the test
- I am using Firefox 47 buildID=20160606114208 and Firefox 50.0a1 buildID=20160614030904
- I use Linux 3.13.0-88-generic x86_64, Qt: 4.8.6, KDE 4.13.3; Kubuntu (trusty) 14.04.4 LTS
- I've searched for duplicates and did not find any.

Marking as NEW

Comment 4

[Gérard Talbot]

Another possible way to test this...:

Reduced test2
-------------
http://www.gtalbot.org/BugzillaSection/Bug1280312-outline-color-invert-outline-shorthand2.html


Reference file2
---------------
http://www.gtalbot.org/BugzillaSection/Bug1280312-outline-color-invert-outline-longhand2.html

The only difference between the reduced test2 and the reference2 is lines 37, 38 and 39.

The browers that support color inversion of pixels will display a filled black square in the test2 and the reference2. 
The browsers that falls back to currentColor will display a filled orange square in the test2 and the reference2. 
And those which do neither will display an unnoticeable white square.

Comment 5

[Gérard Talbot]

Opera 12.16 support color inversion of pixels and renders correctly both tests.

Comment 6

[Gérard Talbot]

Since fall back to currentColor is supported in longhand form, I'm resetting Component to "CSS Parsing and Computation".

Comment 7

[Gérard Talbot]

> I will create a test and submit it to W3C CSS2.1 test suite for inclusion.

Done:

[src; test]
http://test.csswg.org/source/css21/ui/outline-021.xht (shorthand form, shorthand decl.)

[src; test and reference file]
http://test.csswg.org/source/css21/ui/outline-color-176.xht (longhand form, longhand decl.)

outline-color-176 test serves as a reference file for outline-021. So, if one day Firefox supports color inversion of pixels, then the test acting as reference file (outline-color-176) will still be good, reliable as well as their pass-fail-conditions sentences.

Louis, I added a credits comment into the test toward you.

Comment 8

[Louis Lazaris]

(In reply to Gérard Talbot from comment #7)
> > I will create a test and submit it to W3C CSS2.1 test suite for inclusion.
> 
> Done:
> 
> [src; test]
> http://test.csswg.org/source/css21/ui/outline-021.xht (shorthand form,
> shorthand decl.)
> 
> [src; test and reference file]
> http://test.csswg.org/source/css21/ui/outline-color-176.xht (longhand form,
> longhand decl.)
> 
> outline-color-176 test serves as a reference file for outline-021. So, if
> one day Firefox supports color inversion of pixels, then the test acting as
> reference file (outline-color-176) will still be good, reliable as well as
> their pass-fail-conditions sentences.
> 
> Louis, I added a credits comment into the test toward you.

Thank you, that's kind of you! Anything I can do to help.

Comment 9

[Gérard Talbot]

The tests were built last night:

[nightly-unstable]
http://test.csswg.org/suites/css2.1/nightly-unstable/html4/outline-021.htm

[nightly-unstable]
http://test.csswg.org/suites/css2.1/nightly-unstable/html4/outline-color-176.htm

Comment 10

[Thomas Wisniewski]

Attachment: 1280312-support_outline-color:invert_as_currentColor.diff

I'm not sure if fixes for bugs like this are still being accepted with the work on Style having ramped up, but here's a patch adding support for using currentColor for the invert shorthand case. Try seems fine: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e86807a38702

Note that Chrome 53 also fails the shorthand case, WebKit trunk passes both of the tests using the currentColor fallback, and Edge 14 passes both tests by actually inverting the color. Perhaps it's a good time to re-evaluate whether Firefox should support actually inverting the colors as well?

Comment 11

[Cameron McCormack (:heycam)]

Comment on attachment 8792169 [details] [diff] [review]
1280312-support_outline-color:invert_as_currentColor.diff

Unfortunately, I don't think this behaviour is right.  The CSS Basic UI spec says:

  If the UA does not support the invert value then it must reject that value at parse-time,
  and the initial value of the outline-color property is the currentColor [CSS3COLOR] keyword.

I take "reject that value at parse-time" to mean that the use of invert in outline-color or in the outline shorthand is a syntax error, and so the |outline: invert solid 50px| declaration should be dropped, not treated the same as |outline: currentColor solid 50px|.

Comment 12

[Thomas Wisniewski]

I see... if that's the case, then the CSS test in comment 9 is invalid, no? And since the 2.1 test-suite isn't being actively maintained (see Anne's last comment in bug 1119661), I suppose this bug might as well be closed instead, and we'll just have to soak up any interop difference with WebKit until invert is properly supported.

Comment 13

[Gérard Talbot]

(In reply to Cameron McCormack (:heycam) from comment #11)

> Unfortunately, I don't think this behaviour is right.  The CSS Basic UI spec
> says:
> 
>   If the UA does not support the invert value then it must reject that value
> at parse-time,
>   and the initial value of the outline-color property is the currentColor
> [CSS3COLOR] keyword.

https://www.w3.org/TR/css-ui-3/#outline-color

> I take "reject that value at parse-time" to mean that the use of invert in
> outline-color or in the outline shorthand is a syntax error, and so the
> |outline: invert solid 50px| declaration should be dropped, not treated the
> same as |outline: currentColor solid 50px|.

Okay. This is one thing I was afraid of. Anything that is not recognized in the declaration block of a shorthand form makes the whole declaration invalid.

(In reply to Thomas Wisniewski from comment #12)
> I see... if that's the case, then the CSS test in comment 9 is invalid, no?

I'll remove that test then.

> And since the 2.1 test-suite isn't being actively maintained (see Anne's
> last comment in bug 1119661), 

The CSS2.1 test suite is not actively maintained by anyone but test authors had and have a responsibility to check the public css-testsuite mailing list, the Shepherd system and to make appropriate corrections or adjustments to their own tests in a timely manner.

Comment 14

[Gérard Talbot]

A few min. ago, I have been trying to remove
http://test.csswg.org/source/css21/ui/outline-021.xht
and then make small adjustements to
http://test.csswg.org/source/css21/ui/outline-color-176.xht

It seems that Mercurial is no longer supported. So, it may take me some time to untangle all this...

Comment 15

[Thomas Wisniewski]

Thanks for the clarification, Gérard (and for tackling the test changes).

>I'll remove that test then.

Would it not be better to adjust the test so that WebKit "fails" it, rather than having browsers behave differently in this case?

Comment 16

[Gérard Talbot]

> Would it not be better to adjust the test so that WebKit "fails" it, rather
> than having browsers behave differently in this case?

Yes, that is a good idea for later but right now, I am struggling with Mercurial and Git...

Comment 17

[Gérard Talbot]

> I'll remove that test then.

Done.

http://test.csswg.org/source/css21/ui/?C=M;O=D

http://hg.csswg.org/test/rev/7e962a1f8fc9

Comment 18

[Thomas Wisniewski]

Thanks! Marking this bug as INVALID.

Comment 19

[Cameron McCormack (:heycam)]

Thanks Thomas and Gérard.