remove devtools/client/inspector/inspector.css

RESOLVED FIXED in Firefox 51

Status

P2
normal
RESOLVED FIXED
2 years ago
3 months ago

People

(Reporter: gasolin, Assigned: gasolin)

Tracking

unspecified
Firefox 51

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
since there are 2 inspector.css (client/inspector/inspector.css , themes/shared/devtools/inspector.css) in code base, which is confusing,

inspector related styles should be located in themes/shared/devtools/inspector.css
(Assignee)

Comment 1

2 years ago
Patrick, how do you think?
Flags: needinfo?(pbrosset)
The files are:
- devtools/client/inspector/inspector.css
- devtools/client/themes/inspector.css

And they are included by inspector.xul:7,9:
7: <?xml-stylesheet href="chrome://devtools/content/inspector/inspector.css" type="text/css"?>
8: ...
9: <?xml-stylesheet href="chrome://devtools/skin/inspector.css" type="text/css"?>

Seems to me that they are good candidates for merging.
@Fred Lin: Please go ahead and move the CSS from inspector/inspector.css into themes/inspector.css as this is consistent with other tools.

Don't forget to remove the mapping from:
- devtools/client/jar.mn

When you have a patch feel free to ask me for review.
Flags: needinfo?(pbrosset) → needinfo?(gasolin)
Hi Fred,

Please do go ahead with merging the 2, and you can flag me as a a reviewer. We want to keep the copy in devtools/client/themes/inspector.css.

Ideally, I would prefer the rules in the stylesheet to appear in the order of how the elements appear in inspector.xul
Priority: -- → P2
Comment hidden (mozreview-request)
(Assignee)

Comment 6

2 years ago
removed inspector/inspector.css and re-ordered rules by the appearance in inspector.xul
Assignee: nobody → gasolin
Flags: needinfo?(gasolin)

Comment 7

2 years ago
mozreview-review
Comment on attachment 8779601 [details]
Bug 1293591 - remove devtools/client/inspector/inspector.css;

https://reviewboard.mozilla.org/r/70554/#review67936

::: devtools/client/themes/inspector.css:17
(Diff revision 1)
>  }
>  
> +/* Set the minimum width for the side bar so, all tabs are
> +  properly visible. The value can be decreased when bug 1281789
> +  is fixed and the all-tabs-menu is available again. */
> +#inspector-sidebar-container {

#inspector-sidebar-container, #inspector-sidebar, and .inspector-tabpanel > * should come after #inspector-breadcrumbs .breadcrumbs-widget-item in line 132
Attachment #8779601 - Flags: review?(gl) → review+
Comment hidden (mozreview-request)
(Assignee)

Comment 9

2 years ago
issue addressed, thanks!
Status: NEW → ASSIGNED
Keywords: checkin-needed
This needs rebasing. Also, please mark issues in MozReview as resolved when you post a new patch. That'll allow autoland to do its magic :)
Keywords: checkin-needed
Comment hidden (mozreview-request)
(Assignee)

Comment 12

2 years ago
rebased and mark issues in MozReview as resolved, thanks!
Keywords: checkin-needed

Comment 13

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/d83220df469d
Remove devtools/client/inspector/inspector.css. r=gl
Keywords: checkin-needed

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d83220df469d
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51

Updated

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