1293591 - remove devtools/client/inspector/inspector.css

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P2)

Description

[[e-mail]]

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

Comment 1

[[e-mail]]

Patrick, how do you think?

Comment 2

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

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.

Comment 3

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

@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.

Comment 4

[Gabriel Luong [:gl] (ΦωΦ)]

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

Comment 5

[[e-mail]]

Attachment: Bug 1293591 - remove devtools/client/inspector/inspector.css;

Review commit: https://reviewboard.mozilla.org/r/70554/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70554/

Comment 6

[[e-mail]]

removed inspector/inspector.css and re-ordered rules by the appearance in inspector.xul

Comment 7

[Gabriel Luong [:gl] (ΦωΦ)]

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

Comment 8

[[e-mail]]

Comment on attachment 8779601 [details]
Bug 1293591 - remove devtools/client/inspector/inspector.css;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70554/diff/1-2/

Comment 9

[[e-mail]]

issue addressed, thanks!

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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 :)

Comment 11

[[e-mail]]

Comment on attachment 8779601 [details]
Bug 1293591 - remove devtools/client/inspector/inspector.css;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70554/diff/2-3/

Comment 12

[[e-mail]]

rebased and mark issues in MozReview as resolved, thanks!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/d83220df469d
Remove devtools/client/inspector/inspector.css. r=gl

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d83220df469d