Closed
Bug 1270071
Opened 8 years ago
Closed 8 years ago
Missing icons for toggle buttons
Categories
(DevTools :: JSON Viewer, defect)
DevTools
JSON Viewer
Tracking
(Not tracked)
RESOLVED
WORKSFORME
People
(Reporter: Honza, Assigned: Honza)
References
Details
Attachments
(1 file)
6.05 KB,
patch
|
jryans
:
feedback+
|
Details | Diff | Splinter Review |
JSON Viewer is loading icons for the toggle buttons (used to expand/collapse the JSON tree) using chrome:// protocol. This is wrong since chrome://devtools/content is not contentaccessible (I've been always a bit puzzled why this worked before)
The correct way seems to be using resource:// protocol instead.
Honza
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → odvarko
Assignee | ||
Comment 1•8 years ago
|
||
@jryans: what do you think?
- load toggle icons through resource:// protocol
- get theme class name(s) from theme definition
Honza
Attachment #8748601 -
Flags: review?(jryans)
Assignee | ||
Comment 2•8 years ago
|
||
@jryans: some CSS changes are yet needed, but I am curious if this approach looks good to you.
Honza
Comment on attachment 8748601 [details] [diff] [review]
bug1270071.patch
Review of attachment 8748601 [details] [diff] [review]:
-----------------------------------------------------------------
Seems like the right approach overall!
::: devtools/client/jsonview/converter-child.js
@@ +215,5 @@
>
> + // Get theme class name(s) from theme definition.
> + let currentTheme = JsonViewUtils.getCurrentTheme();
> + let themeDef = gDevTools.getThemeDefinition(currentTheme);
> + let themeClassName = themeDef.classList.join(" ");
I would say let's add a new `getCurrentThemeDefinition` to gDevTools to help with this and remove `JsonViewUtils.getCurrentTheme`.
Attachment #8748601 -
Flags: review?(jryans) → feedback+
Updated•8 years ago
|
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•