1266621 - Convert properties which support independent foreground color bit to use the new interpolation behavior

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Xidorn Quan [:xidorn] UTC+10]

In bug 1260543, we convert text-emphasis-color and -webkit-text-fill-color to treat currentcolor as a computed keyword value for transition. We may want to do this as well for other properties which have been using an independent foreground color bit, which is not too hard, but potentially has webcompat issue.

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

The try push of my test patchset [1] shows some reftest failures need to be investigated.

But given dbaron's email to www-style [2], I guess we don't want to do this right now.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca8c629746eb
[2] https://lists.w3.org/Archives/Public/www-style/2016Apr/0379.html

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1266621 part 1 - Convert text-decoration-color to store complex color.

Review commit: https://reviewboard.mozilla.org/r/81346/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81346/

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1266621 part 2 - Convert column-rule-color to store complex color.

Review commit: https://reviewboard.mozilla.org/r/81348/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81348/

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1266621 part 3 - Convert outline-color to store complex color.

Review commit: https://reviewboard.mozilla.org/r/81350/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81350/

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1266621 part 4 - Remove constructors of StyleComplexColor.

So that they can be put inside anonymous structs and unions.

Review commit: https://reviewboard.mozilla.org/r/81352/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81352/

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1266621 part 5 - Convert border-color to store complex color.

Review commit: https://reviewboard.mozilla.org/r/81354/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81354/

Comment 7

[Cameron McCormack (:heycam)]

Comment on attachment 8795256 [details]
Bug 1266621 part 1 - Convert text-decoration-color to store complex color.

https://reviewboard.mozilla.org/r/81346/#review80538

::: layout/style/nsCSSPropList.h:3941
(Diff revision 1)
>      VARIANT_HCK,
>      kBorderColorKTable,

Should we just make this VARIANT_HC to remove support for -moz-use-text-color, and make nsCSSParser::ParseTextDecoration set text-decoration-color to currentColor when it's not present in the shorthand?  (Or as a followup?)  Same for the other properties that accept -moz-use-text-color.

Comment 8

[Cameron McCormack (:heycam)]

Comment on attachment 8795257 [details]
Bug 1266621 part 2 - Convert column-rule-color to store complex color.

https://reviewboard.mozilla.org/r/81348/#review80542

Comment 9

[Cameron McCormack (:heycam)]

Comment on attachment 8795258 [details]
Bug 1266621 part 3 - Convert outline-color to store complex color.

https://reviewboard.mozilla.org/r/81350/#review80546

Comment 10

[Cameron McCormack (:heycam)]

Comment on attachment 8795259 [details]
Bug 1266621 part 4 - Remove constructors of StyleComplexColor.

https://reviewboard.mozilla.org/r/81352/#review80548

Comment 11

[Cameron McCormack (:heycam)]

Comment on attachment 8795260 [details]
Bug 1266621 part 5 - Convert border-color to store complex color.

https://reviewboard.mozilla.org/r/81354/#review80550

r=me with this.

::: layout/generic/nsImageFrame.cpp:1268
(Diff revision 1)
>        // Note: use SetBorderColor here because we want to make sure
>        // the "special" flags are unset.

This comment can be removed.

::: layout/style/nsStyleStruct.h:1304
(Diff revision 1)
>    uint8_t        mBorderImageRepeatH; // [reset] see nsStyleConsts.h
>    uint8_t        mBorderImageRepeatV; // [reset]
>    mozilla::StyleFloatEdge mFloatEdge; // [reset]
>    mozilla::StyleBoxDecorationBreak mBoxDecorationBreak; // [reset]
>  
> +  uint8_t       mBorderStyle[4];  // [reset] See nsStyleConsts.h

I think mBorderStyle should remain protected, so that we are forced to use SetBorderStyle and cause mComputedBorder to be updated.

::: layout/xul/tree/nsTreeBodyFrame.cpp:3265
(Diff revision 1)
>        twistyRect.Inflate(twistyMargin);
>  
>        aRenderingContext.ThebesContext()->Save();
>  
>        const nsStyleBorder* borderStyle = lineContext->StyleBorder();
> -      nscolor color;
> +      // If border didn't touch color, thus grab it from the treeline context

Make this more clear and say "Resolve currentColor values against the treeline context."

Comment 12

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8795256 [details]
Bug 1266621 part 1 - Convert text-decoration-color to store complex color.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81346/diff/1-2/

Comment 13

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8795257 [details]
Bug 1266621 part 2 - Convert column-rule-color to store complex color.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81348/diff/1-2/

Comment 14

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8795258 [details]
Bug 1266621 part 3 - Convert outline-color to store complex color.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81350/diff/1-2/

Comment 15

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8795259 [details]
Bug 1266621 part 4 - Remove constructors of StyleComplexColor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81352/diff/1-2/

Comment 16

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8795260 [details]
Bug 1266621 part 5 - Convert border-color to store complex color.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81354/diff/1-2/

Comment 17

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8795256 [details]
Bug 1266621 part 1 - Convert text-decoration-color to store complex color.

https://reviewboard.mozilla.org/r/81346/#review80538

> Should we just make this VARIANT_HC to remove support for -moz-use-text-color, and make nsCSSParser::ParseTextDecoration set text-decoration-color to currentColor when it's not present in the shorthand?  (Or as a followup?)  Same for the other properties that accept -moz-use-text-color.

Filed bug 1306214.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e99d54b3fabd
part 1 - Convert text-decoration-color to store complex color. r=heycam
https://hg.mozilla.org/integration/autoland/rev/e27b031e2249
part 2 - Convert column-rule-color to store complex color. r=heycam
https://hg.mozilla.org/integration/autoland/rev/1e9cc5be7495
part 3 - Convert outline-color to store complex color. r=heycam
https://hg.mozilla.org/integration/autoland/rev/7b1d4a51d0e8
part 4 - Remove constructors of StyleComplexColor. r=heycam
https://hg.mozilla.org/integration/autoland/rev/022b1fcfe833
part 5 - Convert border-color to store complex color. r=heycam

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e99d54b3fabd
https://hg.mozilla.org/mozilla-central/rev/e27b031e2249
https://hg.mozilla.org/mozilla-central/rev/1e9cc5be7495
https://hg.mozilla.org/mozilla-central/rev/7b1d4a51d0e8
https://hg.mozilla.org/mozilla-central/rev/022b1fcfe833