1274760 - Properties values in the Style Inspector's Rules tab are not styled correctly with the Firebug theme

Status: RESOLVED FIXED

(DevTools :: Inspector: Rules, defect, P1)

Description

[johngraciliano]

User Agent: Mozilla/5.0 (Windows NT 6.2; Win64; x64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20160520030251

Steps to reproduce:

Open a page (e.g., this page), select the Firebug theme for the Developer tools, open the style inspector, rules tab, hover over property values.

Close the developer tools, activate the Firebug extension, inspect the page with Firebug (HTML tool, Style tab).


Actual results:

With the developer tools, the property values are green in FF48 and light purple in FF49. They show a dashed underline when they are hover.

With Firebug the property values are blue and show no underline.


Expected results:

The property values should appear the same.

Comment 1

[johngraciliano]

There are specific rules in chrome://devtools/skin/rules.css to correct this.  They use the selector:

.theme-firebug .ruleview-propertycontainer > .ruleview-propertyvalue

But the selector has is slightly incorrect, it should be:

.theme-firebug .ruleview-propertyvaluecontainer > .ruleview-propertyvalue

Note: one of the rules has a second selector to eliminate the underline under the property name which is correct.

Comment 2

[Patrick Brosset <:pbro>]

Inspector bug triage (filter on CLIMBING SHOES).

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: bug1274760.patch

Patch fixing the issue attached.

Honza

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: rules.png

I am attaching a screenshot that shows how it looks like with the patch (and there is not underline)

@John: does it look ok to you?

Honza

Comment 5

[Patrick Brosset <:pbro>]

Comment on attachment 8760643 [details] [diff] [review]
bug1274760.patch

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

Looks good to me code-wise. Thanks for jumping on this bug Honza.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/2b51d9f20eaa
Fix colors in Rules side panel; r=pbro

Comment 7

[johngraciliano]

One more thing!
The rule that follows at the end of the change:

 .theme-firebug .ruleview-enableproperty:not([checked]) ~ .ruleview-computedlist * {
   color: #CCCCCC;
 }

seems wrong.  The reason for that is that .ruleview-enableproperty and .ruleview-computedlist are not siblings since Firefox 40 (they were siblings in Firefox 39). The rule is intended to make gray the listed (sub) properties from a shorthand property when that shorthand property is disabled.  Given these are not siblings any longer it is hard to fix this with just css.  The best I am able to do is to replace the rule with:

.theme-firebug .ruleview-overridden > * > .ruleview-computed:not(.ruleview-overridden),
.theme-firebug .ruleview-overridden > * > .ruleview-computed:not(.ruleview-overridden) > .ruleview-propertyname,
.theme-firebug .ruleview-overridden > * > .ruleview-computed:not(.ruleview-overridden) > .ruleview-propertyvalue {
  color: #CCCCCC;
}

This works for me but has a problem when disabling a property overridden in another rule.  In such case, it will not apply until inspecting another node.

Comment 8

[johngraciliano]

Attachment: After fixing the rule for .ruleview-computedlist

I am attaching what it should look after the last change I proposed.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2b51d9f20eaa

Comment 10

[johngraciliano]

The patch is good.  The comment I added before has to do with the rule in the line listed as 1.40 in the patch.  That is the rule that follows what was changed in the path and it has to do with shortform properties.  Those will have rules are preceded by a twisty (I also changed it in my picture, but that is a totally different issue).  The rule appeared to have the intention to make the included properties listed turn gray.  As I stated it does not work (at least for me) because of the new tree structure of the code.  I proposed an alternative for that.

The rule I proposed missing a selector.  It should read:
.theme-firebug .ruleview-overridden > * > .ruleview-computed:not(.ruleview-overridden),
.theme-firebug .ruleview-overridden > * > .ruleview-computed:not(.ruleview-overridden) > .ruleview-propertyname,
.theme-firebug .ruleview-overridden > * > .ruleview-computed:not(.ruleview-overridden) > .ruleview-propertyvalue,
.theme-firebug .ruleview-overridden > * > .ruleview-computed:not(.ruleview-overridden) > .ruleview-propertyvalue > .theme-link {
  color: #CCCCCC;
}

However it can be posibly simplified to:
.theme-firebug .ruleview-overridden > * > .ruleview-computed:not(.ruleview-overridden),
.theme-firebug .ruleview-overridden > * > .ruleview-computed:not(.ruleview-overridden) * {
  color: #CCCCCC;
}
This last version is consistent with the other rules that preceed it.  However I understand selectors that end with '*' should be avoided.