Rule view shows doesn't distinguish an invalid rgb color in a text-shadow rule

RESOLVED FIXED

Status

DevTools
Inspector
RESOLVED FIXED
5 years ago
5 days ago

People

(Reporter: pqwoerituytrueiwoq, Assigned: tromey)

Tracking

25 Branch
x86_64
Linux

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [rule-view])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:25.0) Gecko/20130722 Firefox/25.0 (Nightly/Aurora)
Build ID: 20130722030226

Steps to reproduce:

64bit Firefox Nightly for linux: 25.0a1 (2013-07-22)

Right Click -> Inspect Element (one with a text-shadow)


Actual results:

Text shadow was reported as:
text-shadow: 0px 0px 1px, 0px 0px 2px, 0px 0px 3px, 0px 0px 4px;


Expected results:

Text shadow really is:
text-shadow: 0px 0px 1px rgb(125,125,125), 0px 0px 2px rgb(125,125,125, 0px 0px 3px, 0px 0px 4px rgb(125,125,125);
(Reporter)

Comment 1

5 years ago
* Note there was not a typo on the 3ed value under really is
(Reporter)

Updated

5 years ago
Component: Untriaged → Developer Tools: Style Editor
(Reporter)

Comment 2

5 years ago
not a 100% reproducible it seems, the style was in effect and working
(Reporter)

Comment 3

5 years ago
Ok, floating point values in rgb break the dev tool
rgb(127.5,127.5,127.5)

Comment 4

5 years ago
Floating point values are not allowed in rgb(), except the alpha value in rgba().
These values actually work, so we should show them too. Although, I believe that its the rule view that is being referenced here, not the style editor.
Status: UNCONFIRMED → NEW
Component: Developer Tools: Style Editor → Developer Tools: Inspector
Ever confirmed: true
Whiteboard: [rule-view]
Basic test case showing the reported issue: http://fiddle.jshell.net/bgrins/mYMKB/show/.  pbrosset was working on a patch that allows authored styles in the rule view, which would likely solve this problem - I can't find a bug # off hand though.  Patrick, is there a bug opened for this?
Flags: needinfo?(pbrosset)
Indeed as bgrins noted, this is a problem of our rule-view not showing authored styles, but rather actually applied styles.
I haven't yet started working on my authored-style support demo. I created bug 984880 for this.
Flags: needinfo?(pbrosset)
Wha tI am wondering is that the invalid rgb values actually do apply, I see blue color if I do something like rgb(1.1, 1.2, 20) . So rule view should show at least some values !
(In reply to Girish Sharma [:Optimizer] from comment #8)
> Wha tI am wondering is that the invalid rgb values actually do apply, I see
> blue color if I do something like rgb(1.1, 1.2, 20) . So rule view should
> show at least some values !
Really? I my tests, if a RGB color has floating point values, it just doesn't get applied.
So, I think the rule-view works "as expected" considering it doesn't deal with authored styles as this stage.
(In reply to Girish Sharma [:Optimizer] from comment #8)
> Wha tI am wondering is that the invalid rgb values actually do apply, I see
> blue color if I do something like rgb(1.1, 1.2, 20) . So rule view should
> show at least some values !

RGB colors by spec are integers or percentages: http://www.w3.org/TR/css3-color/#rgb-color.  "The format of an RGB value in the functional notation is ‘rgb(’ followed by a comma-separated list of three numerical values (either three integer values or three percentage values) followed by ‘)’"

What do you see if you do window.getComputedStyle for text-shadow on this element?
See the screenshot, I added decimal values, they did not appear, but got applied : http://i.snag.gy/wF9nX.jpg
I think the shadow got applied, but not the color. Try choosing a color that's easy to recognize and using the offset to see it better. Something like:

10px 10px 0px rgb(1.1, 1.1, 200);

you will see that the shadow has the default color, and is not blue.
But this:

10px 10px 0px rgb(1, 1, 200);

will produce a blue shadow.
(Reporter)

Comment 13

4 years ago
on Brians's jsfiddle, window.getComputedStyle(document.getElementById('cat'),null).getPropertyValue('text-shadow') returned:
rgb(0, 0, 0) 0px 0px 1px

**The ID of cat was added via firebug
(Reporter)

Comment 14

4 years ago
I just confired what Patrick thought, the decimal values make it fall back to 0,0,0
this was on firefox 27.0.1
So we should've at least shown that in the rule view as per the current logic of getting the computed values from gecko. :)
(Assignee)

Comment 16

3 years ago
With the as-authored series applied, the invalid value does
show up in the rule view.  E.g., for the link in comment #6
I see:

text-shadow: 0px 0px 1px rgb(127.5,127.5,127.5);

It is a bit misleading here because the rgb() is not applied.
There is one subtle difference here, though, which is that the
invalid rgb doesn't have a color swatch -- but I wouldn't expect
most users to notice this.

It's also clear that the color isn't applied by consulting the
computed view.

One way we could perhaps do better would be to change output-parser
to strike-through invalid colors (not names of course, but rgb,
hsl, or hex).
Depends on: 984880
(Assignee)

Comment 17

3 years ago
Created attachment 8671618 [details]
Screenshot from 2015-10-08 15-18-09.png

Here's what the line-through idea looks like.
(Assignee)

Comment 18

3 years ago
Created attachment 8671620 [details] [diff] [review]
strike through text that looks like a color but is not quite valid

Patch I used to make that screenshot.
(Assignee)

Comment 19

3 years ago
Brian, do you think this approach is worth pursuing?
Flags: needinfo?(bgrinstead)
(In reply to Tom Tromey :tromey from comment #19)
> Brian, do you think this approach is worth pursuing?

Yeah, that looks great
Flags: needinfo?(bgrinstead)
(Assignee)

Updated

3 years ago
Assignee: nobody → ttromey
Status: NEW → ASSIGNED
Summary: Developer tools does not show text-shadow color → Rule view shows doesn't distinguish an invalid rgb color in a text-shadow rule
(Assignee)

Comment 21

3 years ago
Created attachment 8672009 [details] [diff] [review]
strike through text that looks like a color but is not quite valid
Attachment #8671620 - Attachment is obsolete: true
(Assignee)

Comment 22

3 years ago
Created attachment 8672012 [details] [diff] [review]
strike through text that looks like a color but is not quite valid

Now with debugging removed.
Attachment #8672009 - Attachment is obsolete: true
(Assignee)

Comment 23

3 years ago
Comment on attachment 8672012 [details] [diff] [review]
strike through text that looks like a color but is not quite valid

Here's the patch plus tests.

I considered having this look for invalid hex colors as well but I didn't think that would be as useful.
Attachment #8672012 - Flags: review?(bgrinstead)
Comment on attachment 8672012 [details] [diff] [review]
strike through text that looks like a color but is not quite valid

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

This looks good to me
Attachment #8672012 - Flags: review?(bgrinstead) → review+
(Assignee)

Comment 26

3 years ago
Something has changed here since I wrote the patch, because now
DOMUtils.cssPropertyIsValid returns false for the text-shadow in
the example.  The property isn't applied and appears with a line
through it (and a warning sign) in the rule view.

I don't know of another way to reproduce this problem, but I'm looking.
(Assignee)

Comment 27

3 years ago
I think this was fixed by bug 1199610 and that we don't actually need this patch.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED

Updated

5 days ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.