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)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: xidorn, Assigned: xidorn)
References
Details
(Keywords: dev-doc-complete, site-compat)
Attachments
(3 files)
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 | ||
Updated•7 years ago
|
Keywords: dev-doc-needed,
site-compat
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → xidorn+moz
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=210db9f12c30
Comment 5•7 years ago
|
||
mozreview-review |
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 6•7 years ago
|
||
mozreview-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+
Assignee | ||
Comment 7•7 years ago
|
||
mozreview-review-reply |
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 8•7 years ago
|
||
mozreview-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+
Assignee | ||
Comment 9•7 years ago
|
||
mozreview-review-reply |
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 "!=".
Assignee | ||
Comment 10•7 years ago
|
||
mozreview-review-reply |
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
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 14•7 years ago
|
||
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
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d0572bb7e28d https://hg.mozilla.org/mozilla-central/rev/fde14639c022 https://hg.mozilla.org/mozilla-central/rev/5381a4a1fefa
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment 16•7 years ago
|
||
Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/moz-use-text-color-css-colour-keyword-has-been-dropped/
Assignee | ||
Comment 17•7 years ago
|
||
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.
Comment 18•7 years ago
|
||
Thanks, updated the doc. Looks like the Gecko source is also still saying "currentColor" ;)
Assignee | ||
Comment 19•7 years ago
|
||
(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 :)
Comment 20•6 years ago
|
||
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.
Description
•