1255975 - tree-view.css uses firebug images and as wrong reference to theme-light.css

Status: RESOLVED FIXED

(DevTools :: Shared Components, defect, P2)

Description

[Alfred Kayser]

From bug 1201475:
theme-light.css should be light-theme.css

tree-view.css still refer to firebug specific images:
77 /* Spinner (used for async fetch). Needs to have higher priority than
78    theme toggle icons */
79 .treeTable .treeRow.hasChildren.loading > .treeLabelCell > .treeIcon {
80   background-image: url(chrome://devtools/skin/images/firebug/spinner.png) !important;
81   background-position: 2px 1px !important;
82   background-size: 9px 9px !important;
83 }
84 
85 /* Default toggle icon. The immediate children operator must be
86  used here since there might be nested tree components inside
87  a tree and we don't want to alter those. */
88 .treeTable .treeRow.hasChildren > .treeLabelCell > .treeIcon {
89   background-image: url(chrome://devtools/skin/images/firebug/twisty-closed-firebug.svg);
90 }
91 
92 .treeTable .treeRow.hasChildren.opened > .treeLabelCell > .treeIcon {
93   background-image: url(chrome://devtools/skin/images/firebug/twisty-open-firebug.svg);
94 }

Comment 1

[:Gijs (he/him)]

The light-theme.css import should have gotten you backed out. It's failing the CSS mochitest-bc test on my local machine; I don't know why it's not failing in infra, but it should be addressed. Honza, can you take this?

Comment 2

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

Attachment: bug1255975.patch

@Gijs: does this fix the failing test for you?

Honza

Comment 3

[:Gijs (he/him)]

Comment on attachment 8731790 [details] [diff] [review]
bug1255975.patch

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

Yes, this fixes it.

Comment 4

[:Gijs (he/him)]

(note that this bug was also filed because there are firebug images referenced that presumably also don't exist; we should not mark the bug fixed until that is fixed, too - or file a followup.)

Comment 5

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

There are three images mentioned in comment #0
chrome://devtools/skin/images/firebug/twisty-closed-firebug.svg
chrome://devtools/skin/images/firebug/twisty-open-firebug.svg
chrome://devtools/skin/images/firebug/spinner.png

All these files exist (they are part of Firebug theme, wip)

Or, are you referring to another set of files?

Honza

Comment 6

[:Gijs (he/him)]

(In reply to Jan Honza Odvarko [:Honza] from comment #5)
> There are three images mentioned in comment #0
> chrome://devtools/skin/images/firebug/twisty-closed-firebug.svg
> chrome://devtools/skin/images/firebug/twisty-open-firebug.svg
> chrome://devtools/skin/images/firebug/spinner.png
> 
> All these files exist (they are part of Firebug theme, wip)
> 
> Or, are you referring to another set of files?
> 
> Honza

No, that's it. I must have misunderstood comment 0 - I guess the complaint is that CSS files that we ship as part of content/code/components shouldn't rely on particular images that are part of themes/skin, because that means third-party theme authors have to ship the same images at those paths (or override the style rules specifically to produce alternative images). If the images are required for the tree-view to function, they should be shipped under resource: / chrome://.../content/, not under chrome://.../skin/ .

Comment 7

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

(In reply to :Gijs Kruitbosch from comment #6)
> No, that's it. I must have misunderstood comment 0 - I guess the complaint
> is that CSS files that we ship as part of content/code/components shouldn't
> rely on particular images that are part of themes/skin, because that means
> third-party theme authors have to ship the same images at those paths (or
> override the style rules specifically to produce alternative images). If the
> images are required for the tree-view to function, they should be shipped
> under resource: / chrome://.../content/, not under chrome://.../skin/ .
OK, I see. I believe I'll be looking at this when fixing bug 1244054.

Honza

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/35ba5b2bb52b

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/35ba5b2bb52b