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

RESOLVED FIXED in Firefox 48

Status

P2
normal
RESOLVED FIXED
3 years ago
4 months ago

People

(Reporter: alfredkayser, Assigned: Honza)

Tracking

unspecified
Firefox 48

Firefox Tracking Flags

(firefox48 fixed)

Details

(Whiteboard: [btpp-fix-later])

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
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 }
(Reporter)

Updated

3 years ago
Blocks: 1247065
Component: Developer Tools → Developer Tools: Shared Components
Priority: -- → P2
Whiteboard: [btpp-fix-later]

Comment 1

3 years ago
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?
Flags: needinfo?(odvarko)
(Assignee)

Comment 2

3 years ago
Created attachment 8731790 [details] [diff] [review]
bug1255975.patch

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

Honza
Assignee: nobody → odvarko
Status: NEW → ASSIGNED
Flags: needinfo?(odvarko)
Attachment #8731790 - Flags: review?(gijskruitbosch+bugs)

Comment 3

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

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

Yes, this fixes it.
Attachment #8731790 - Flags: review?(gijskruitbosch+bugs) → review+

Comment 4

3 years ago
(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.)
(Assignee)

Comment 5

3 years ago
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
Flags: needinfo?(gijskruitbosch+bugs)

Comment 6

3 years ago
(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/ .
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 7

3 years ago
(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
Keywords: checkin-needed

Comment 9

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/35ba5b2bb52b
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48

Updated

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