1201136 - Inspector doesn't support the currentColor keyword

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Attachment: currentColor-bug.gif

This seems like a validation problem in the ruleview, see attached .gif.

STR: 

Apply the rule `background-color: currentColor` to any element.

ER: the currentColor value should be visible

AR: it's invisible! The color however is actually applied and the markupview is updated.


Aside: currentColor is a valid keyword for color-related rules, it should probably also be added to the autocomplete list of named colors you see in the .gif.

Comment 1

[Tom Tromey :tromey]

I think the bug is in output-parser.js.
_appendColor returns a boolean but nothing ever checks it.
It seems I introduced this.

Comment 2

[Tom Tromey :tromey]

inDOMUtils doesn't include "currentColor" in its list of completions (it just
gives the list from nsColorNameList.h, which includes "transparent"); but
there is a catch-all to add "inherit", "initial", and "unset".  So "currentColor"
seems to be the only missing one.

Comment 3

[Patrick Brosset <:pbro>]

We should also check (maybe another bug) what it would take to show the color swatch next to currentColor, knowing that it's a dynamic variable that depends on the inherited value of color, so we might need to do some tricky refreshing.

Comment 4

[Tom Tromey :tromey]

Attachment: treat "currentColor" as a valid color

This patch fixes the completion problem; and updates
test_is_valid_css_color.html to cover the "transparent" and
"currentColor" cases for good measure.

This doesn't fix the entire problem.

Comment 5

[Tom Tromey :tromey]

(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #3)
> We should also check (maybe another bug) what it would take to show the
> color swatch next to currentColor, knowing that it's a dynamic variable that
> depends on the inherited value of color, so we might need to do some tricky
> refreshing.

Totally agreed; to me this sounds like bug 1102464.

Comment 6

[Tom Tromey :tromey]

Attachment: make output-parser correctly handle special color values

Fix output-parser to keep the text when a color swatch isn't
appropriate.

Comment 7

[Tom Tromey :tromey]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=af3f3201e48c

Comment 8

[Brian Grinstead [:bgrins]]

Comment on attachment 8656147 [details] [diff] [review]
make output-parser correctly handle special color values

Review of attachment 8656147 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/shared/test/browser_outputparser.js
@@ +99,5 @@
>                     "blur(1px) drop-shadow(0 0 0 ",
>                     {name: "blue", value: "#00F"},
> +                   ") url(\"red.svg#blue\")</span></span>"]),
> +
> +    makeColorTest("color", "currentColor", ["currentColor"])

Uber nit: if you add a comma at the end of this line then it's blame won't be affected when the next test case is added.

Comment 9

[Tom Tromey :tromey]

Attachment: make output-parser correctly handle special color values

Updated per review.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/5c6c10173e6e
https://hg.mozilla.org/integration/fx-team/rev/d9b48d545c2b

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5c6c10173e6e
https://hg.mozilla.org/mozilla-central/rev/d9b48d545c2b

Comment 12

[Simon Lindholm]

Note also the comment from a while ago in bug 927367, that system colors like ActiveBorder, ActiveCaption, etc. are also missing from auto-completion. Certainly not as big a deal, given their obscurity and that they still show in the rule view. In fact, maybe one does not even want to surface them in auto-completion...

Comment 13

[Tom Tromey :tromey]

(In reply to Simon Lindholm from comment #12)
> Note also the comment from a while ago in bug 927367, that system colors
> like ActiveBorder, ActiveCaption, etc. are also missing from
> auto-completion. Certainly not as big a deal, given their obscurity and that
> they still show in the rule view. In fact, maybe one does not even want to
> surface them in auto-completion...

Yes, I talked to Patrick about this and I think we don't want to expose these.
So I am going to close that bug as well.