Closed Bug 1366723 Opened 3 years ago Closed 3 years ago

netmonitor.css should be reference to where it lives

Categories

(DevTools :: Netmonitor, enhancement, P3)

enhancement

Tracking

(firefox55 fixed)

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- fixed

People

(Reporter: ochameau, Assigned: Honza)

References

Details

Attachments

(1 file)

netmonitor.css is the only one file in DevTools jar that involves a build-time magic layout. i.e. its URL is different from where it actually lives.

It is bad for two reasons:
1) It is harder to figure out where the css is.
http://searchfox.org/mozilla-central/source/devtools/client/netmonitor/index.html#9
<link rel="stylesheet" href="chrome://devtools/skin/netmonitor.css"/>
You would imagine finding it next to index.html, but no.

2) It won't work in DevTools as an add-on, as there is no build step and files URL matches the source tree.

So we should get rid of this:
  skin/netmonitor.css (netmonitor/src/assets/styles/netmonitor.css)
in favor of:
  skin/netmonitor/src/assets/styles/netmonitor.css (netmonitor/src/assets/styles/netmonitor.css)
http://searchfox.org/mozilla-central/source/devtools/client/jar.mn#159

Or move netmonitor.css next to index.html in /devtools/client/netmonitor/.

Honza, could you make the call?
Flags: needinfo?(odvarko)
Flags: needinfo?(odvarko)
Priority: -- → P3
Assignee: nobody → odvarko
Status: NEW → ASSIGNED
Comment on attachment 8870433 [details]
Bug 1366723 - Fix URL of netmonitor.css

https://reviewboard.mozilla.org/r/141874/#review145544

::: devtools/client/jar.mn:159
(Diff revision 1)
>      skin/images/breadcrumbs-scrollbutton.png (themes/images/breadcrumbs-scrollbutton.png)
>      skin/images/breadcrumbs-scrollbutton@2x.png (themes/images/breadcrumbs-scrollbutton@2x.png)
>      skin/animationinspector.css (themes/animationinspector.css)
>      skin/canvasdebugger.css (themes/canvasdebugger.css)
>      skin/debugger.css (themes/debugger.css)
> -    skin/netmonitor.css (netmonitor/src/assets/styles/netmonitor.css)
> +    skin/netmonitor/src/assets/styles/netmonitor.css (netmonitor/src/assets/styles/netmonitor.css)

This is still a "magic" mapping.
If you want to keep it under the "skin" url (chrome://devtools/skin/), you would have to move netmonitor.css under client/themes/... folder.
I think we should just drop this skin reference and pull it from the "content" url (chrome://devtools/content).

::: devtools/client/netmonitor/index.html:9
(Diff revision 1)
>  <!DOCTYPE html>
>  <html dir="">
>    <head>
>      <link rel="stylesheet" href="chrome://devtools/content/shared/widgets/widgets.css"/>
>      <link rel="stylesheet" href="chrome://devtools/skin/widgets.css"/>
> -    <link rel="stylesheet" href="chrome://devtools/skin/netmonitor.css"/>
> +    <link rel="stylesheet" href="chrome://devtools/skin/netmonitor/src/assets/styles/netmonitor.css"/>

So here it should be either:
chrome://devtools/content/netmonitor/src/assets/styles/netmonitor.css
or
./src/assets/styles/netmonitor.css
Attachment #8870433 - Flags: review?(poirot.alex)
Comment on attachment 8870433 [details]
Bug 1366723 - Fix URL of netmonitor.css

https://reviewboard.mozilla.org/r/141874/#review147540

Looks good now. Thanks Honza!
Attachment #8870433 - Flags: review?(poirot.alex) → review+
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c605d84491f8
Fix URL of netmonitor.css r=ochameau
https://hg.mozilla.org/mozilla-central/rev/c605d84491f8
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.