netmonitor.css should be reference to where it lives

RESOLVED FIXED in Firefox 55

Status

P3
normal
RESOLVED FIXED
a year ago
2 months ago

People

(Reporter: ochameau, Assigned: Honza)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 55

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

a year ago
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?
(Reporter)

Updated

a year ago
Flags: needinfo?(odvarko)
(Assignee)

Updated

a year ago
Flags: needinfo?(odvarko)
Priority: -- → P3
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Assignee: nobody → odvarko
Status: NEW → ASSIGNED
(Reporter)

Comment 2

a year ago
mozreview-review
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 hidden (mozreview-request)
(Reporter)

Comment 4

a year ago
mozreview-review
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+

Comment 5

a year ago
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c605d84491f8
Fix URL of netmonitor.css r=ochameau

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c605d84491f8
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

Updated

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