Closed Bug 1266621 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

Attachments

(5 files)

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.
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
Assignee: nobody → xidorn+moz
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.
Attachment #8795256 - Flags: review?(cam) → review+
Comment on attachment 8795257 [details]
Bug 1266621 part 2 - Convert column-rule-color to store complex color.

https://reviewboard.mozilla.org/r/81348/#review80542
Attachment #8795257 - Flags: review?(cam) → review+
Blocks: 1306214
Comment on attachment 8795258 [details]
Bug 1266621 part 3 - Convert outline-color to store complex color.

https://reviewboard.mozilla.org/r/81350/#review80546
Attachment #8795258 - Flags: review?(cam) → review+
Comment on attachment 8795259 [details]
Bug 1266621 part 4 - Remove constructors of StyleComplexColor.

https://reviewboard.mozilla.org/r/81352/#review80548
Attachment #8795259 - Flags: review?(cam) → review+
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."
Attachment #8795260 - Flags: review?(cam) → review+
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.
Pushed by xquan@mozilla.com:
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
See Also: → 1400779
You need to log in before you can comment on or make changes to this bug.