Closed Bug 1306214 Opened 7 years ago Closed 7 years ago

Remove -moz-use-text-color from color properties

Categories

(Core :: CSS Parsing and Computation, defect)

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

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla52
Tracking Flags:
Tracking Status
firefox52 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(3 files)

Bug 1306214 part 1 - Make mach devtools-css-db windows-compatible.
58 bytes, text/x-review-board-request
tromey
: review+
Details
Bug 1306214 part 2 - Remove -moz-use-text-color from devtools.
58 bytes, text/x-review-board-request
tromey
: review+
Details
Bug 1306214 part 3 - Remove -moz-use-text-color from Gecko.
58 bytes, text/x-review-board-request
heycam
: review+
Details 
After bug 1266621, -moz-use-text-color is almost equivalent to currentcolor. We should remove that value, and switch any use of that to currentcolor instead.
Assignee: nobody → xidorn+moz
Comment on attachment 8796120 [details]
Bug 1306214 part 1 - Make mach devtools-css-db windows-compatible.

https://reviewboard.mozilla.org/r/82088/#review80650

Thank you.  Looks good.
Attachment #8796120 - Flags: review?(ttromey) → review+
Comment on attachment 8796121 [details]
Bug 1306214 part 2 - Remove -moz-use-text-color from devtools.

https://reviewboard.mozilla.org/r/82090/#review80656

Thank you.

This is fine with the issue addressed; and if the main patch to remove -moz-use-text-color
from platform is also approved.

::: devtools/client/debugger/new/bundle.js:4625
(Diff revision 1)
>  	    values: ["-moz-calc", "auto", "calc", "inherit", "initial", "unset", ],
>  	  },
>  	  "border-block-end-color": {
>  	    inherited: false,
>  	    supports: 4,
> -	    values: ["-moz-use-text-color", "aliceblue", "antiquewhite", "aqua", "aquamarine", "azure", "beige", "bisque", "black", "blanchedalmond", "blue", "blueviolet", "brown", "burlywood", "cadetblue", "chartreuse", "chocolate", "coral", "cornflowerblue", "cornsilk", "crimson", "currentColor", "cyan", "darkblue", "darkcyan", "darkgoldenrod", "darkgray", "darkgreen", "darkgrey", "darkkhaki", "darkmagenta", "darkolivegreen", "darkorange", "darkorchid", "darkred", "darksalmon", "darkseagreen", "darkslateblue", "darkslategray", "darkslategrey", "darkturquoise", "darkviolet", "deeppink", "deepskyblue", "dimgray", "dimgrey", "dodgerblue", "firebrick", "floralwhite", "forestgreen", "fuchsia", "gainsboro", "ghostwhite", "gold", "goldenrod", "gray", "grey", "green", "greenyellow", "honeydew", "hotpink", "hsl", "hsla", "indianred", "indigo", "inherit", "initial", "ivory", "khaki", "lavender", "lavenderblush", "lawngreen", "lemonchiffon", "lightblue", "lightcoral", "lightcyan", "lightgoldenrodyellow", "lightgray", "lightgreen", "lightgrey", "lightpink", "lightsalmon", "lightseagreen", "lightskyblue", "lightslategray", "lightslategrey", "lightsteelblue", "lightyellow", "lime", "limegreen", "linen", "magenta", "maroon", "mediumaquamarine", "mediumblue", "mediumorchid", "mediumpurple", "mediumseagreen", "mediumslateblue", "mediumspringgreen", "mediumturquoise", "mediumvioletred", "midnightblue", "mintcream", "mistyrose", "moccasin", "navajowhite", "navy", "oldlace", "olive", "olivedrab", "orange", "orangered", "orchid", "palegoldenrod", "palegreen", "paleturquoise", "palevioletred", "papayawhip", "peachpuff", "peru", "pink", "plum", "powderblue", "purple", "rebeccapurple", "red", "rgb", "rgba", "rosybrown", "royalblue", "saddlebrown", "salmon", "sandybrown", "seagreen", "seashell", "sienna", "silver", "skyblue", "slateblue", "slategray", "slategrey", "snow", "springgreen", "steelblue", "tan", "teal", "thistle", "tomato", "transparent", "turquoise", "unset", "violet", "wheat", "white", "whitesmoke", "yellow", "yellowgreen", ],
> +	    values: ["aliceblue", "antiquewhite", "aqua", "aquamarine", "azure", "beige", "bisque", "black", "blanchedalmond", "blue", "blueviolet", "brown", "burlywood", "cadetblue", "chartreuse", "chocolate", "coral", "cornflowerblue", "cornsilk", "crimson", "currentColor", "cyan", "darkblue", "darkcyan", "darkgoldenrod", "darkgray", "darkgreen", "darkgrey", "darkkhaki", "darkmagenta", "darkolivegreen", "darkorange", "darkorchid", "darkred", "darksalmon", "darkseagreen", "darkslateblue", "darkslategray", "darkslategrey", "darkturquoise", "darkviolet", "deeppink", "deepskyblue", "dimgray", "dimgrey", "dodgerblue", "firebrick", "floralwhite", "forestgreen", "fuchsia", "gainsboro", "ghostwhite", "gold", "goldenrod", "gray", "grey", "green", "greenyellow", "honeydew", "hotpink", "hsl", "hsla", "indianred", "indigo", "inherit", "initial", "ivory", "khaki", "lavender", "lavenderblush", "lawngreen", "lemonchiffon", "lightblue", "lightcoral", "lightcyan", "lightgoldenrodyellow", "lightgray", "lightgreen", "lightgrey", "lightpink", "lightsalmon", "lightseagreen", "lightskyblue", "lightslategray", "lightslategrey", "lightsteelblue", "lightyellow", "lime", "limegreen", "linen", "magenta", "maroon", "mediumaquamarine", "mediumblue", "mediumorchid", "mediumpurple", "mediumseagreen", "mediumslateblue", "mediumspringgreen", "mediumturquoise", "mediumvioletred", "midnightblue", "mintcream", "mistyrose", "moccasin", "navajowhite", "navy", "oldlace", "olive", "olivedrab", "orange", "orangered", "orchid", "palegoldenrod", "palegreen", "paleturquoise", "palevioletred", "papayawhip", "peachpuff", "peru", "pink", "plum", "powderblue", "purple", "rebeccapurple", "red", "rgb", "rgba", "rosybrown", "royalblue", "saddlebrown", "salmon", "sandybrown", "seagreen", "seashell", "sienna", "silver", "skyblue", "slateblue", "slategray", "slategrey", "snow", "springgreen", "steelblue", "tan", "teal", "thistle", "tomato", "transparent", "turquoise", "unset", "violet", "wheat", "white", "whitesmoke", "yellow", "yellowgreen", ],

This file is maintained out of tree, so it shouldn't be touched by your patch.

Since it's the new debugger, I don't think it actively does anything with the various CSS properties.  So, I think it's harmless to just drop this from your patch.

Please file a bug here: https://github.com/devtools-html/debugger.html/issues

That will ensure that some future import of the debugger from github will fix this problem.
Attachment #8796121 - Flags: review?(ttromey) → review+
Comment on attachment 8796121 [details]
Bug 1306214 part 2 - Remove -moz-use-text-color from devtools.

https://reviewboard.mozilla.org/r/82090/#review80656

> This file is maintained out of tree, so it shouldn't be touched by your patch.
> 
> Since it's the new debugger, I don't think it actively does anything with the various CSS properties.  So, I think it's harmless to just drop this from your patch.
> 
> Please file a bug here: https://github.com/devtools-html/debugger.html/issues
> 
> That will ensure that some future import of the debugger from github will fix this problem.

OK, I'll file one once the final patch gets r+. Thanks for your review.
Comment on attachment 8796122 [details]
Bug 1306214 part 3 - Remove -moz-use-text-color from Gecko.

https://reviewboard.mozilla.org/r/82092/#review80668

::: layout/style/nsCSSParser.cpp:13482
(Diff revision 1)
> -      if (ParseVariant(cur->mValue, VARIANT_COLOR | VARIANT_KEYWORD,
> -                       nsCSSProps::kBorderColorKTable) != CSSParseResult::Ok) {
> +      if (ParseVariant(cur->mValue, VARIANT_COLOR,
> +                       nullptr) != CSSParseResult::Ok) {

Nit: these lines might have a more natural wrapping point, now.

::: layout/style/nsStyleContext.h:407
(Diff revision 1)
>     * Note that if aProperty is eCSSProperty_border_*_color, this
> -   * function handles -moz-use-text-color.
> +   * function handles currentcolor.

Since GetVisitedDependentColor handles currentcolor -> nscolor conversion for all properties it accepts, can you either remove this sentence or improve it?
Attachment #8796122 - Flags: review?(cam) → review+
Comment on attachment 8796122 [details]
Bug 1306214 part 3 - Remove -moz-use-text-color from Gecko.

https://reviewboard.mozilla.org/r/82092/#review80668

> Nit: these lines might have a more natural wrapping point, now.

OK, although I do prefer the current wrapper, I'm fine with making it break after "!=".
Comment on attachment 8796121 [details]
Bug 1306214 part 2 - Remove -moz-use-text-color from devtools.

https://reviewboard.mozilla.org/r/82090/#review80656

> OK, I'll file one once the final patch gets r+. Thanks for your review.

Filed https://github.com/devtools-html/debugger.html/issues/848
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d0572bb7e28d
part 1 - Make mach devtools-css-db windows-compatible. r=tromey
https://hg.mozilla.org/integration/autoland/rev/fde14639c022
part 2 - Remove -moz-use-text-color from devtools. r=tromey
https://hg.mozilla.org/integration/autoland/rev/5381a4a1fefa
part 3 - Remove -moz-use-text-color from Gecko. r=heycam
Looks like the bug you reference in this post is wrong?

Also this value is not expected to be widely used as it was mainly a hack for the initial value of those properties before currentcolor is defined as its current behavior.

BTW, currentcolor should be "currentcolor" not "currentColor". I guess various documents need to be updated for this.
Thanks, updated the doc. Looks like the Gecko source is also still saying "currentColor" ;)
(In reply to Kohei Yoshino [:kohei] from comment #18)
> Thanks, updated the doc. Looks like the Gecko source is also still saying
> "currentColor" ;)

I guess we want to gradually change that :)
I only found one mention of this proprietary color keyword on MDN:

https://developer.mozilla.org/en-US/docs/Web/CSS/Mozilla_Extensions

Here I've marked it as obsolete and recommended using currentColor instead.

I also added a note to the Fx 52 release notes:

https://developer.mozilla.org/en-US/Firefox/Releases/52#CSS
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.