971191 - Style sheet name display should contain path from root

Status: NEW

(DevTools :: Style Editor, defect, P2)

Description

[Heather Arthur [:harth]]

STR:
1) Visit http://harthur.github.io/wwcode/
2) Open Style Editor

Actual:
sources are "normalize.css", "wwcode.css"

Expected:
sources are "css/normalize.css", "css/wwcode.css"

Comment 1

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

After talking to Heather, this bug is actually a regression where the original labelling was lost. The expected css source labels should be appended with the directory one level up. The old code can be found here: https://hg.mozilla.org/releases/mozilla-beta/file/f0ec5536ce00/browser/devtools/styleeditor/StyleSheetEditor.jsm#l129

Comment 2

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

Attachment: 971191.patch

This patch changes the following:
- Brought back the code lost in the Source Map regression. Since we lost debuggee, I used this.styleSheet.nodeHref to substitute for the href path up to the root of the css files.
eg) Given http://harthur.github.io/wwcode/css/normalize.css
nodeHref = "http://harthur.github.io/wwcode/"
- Needed some special handling for original sources since they don't have nodeHref. For now, their file name is returned, but I think we might want to do something here.
- This also fixes Bug 949673 - Imported sheets should show last part of url as name, not "css". This is resolved by simply returning the sheetURI if no nodeHref was parsed.
- System css sources (eg, chrome://global/skin/global.css) behaves a bit weirdly in the Browser Toolbox. Only some sources have nodeHref and others do not. So, if they have a nodeHref, a pretty name appears, otherwise, we get their sheetURI. This seems consistent with Firefox Release since the sheetURI appears if the start page was inspected.
- Existing StyleEditor tests passed

Let me know if you have any feedback. Thanks!

Comment 3

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

Attachment: Inconsistency with the system css source file naming in the toolbox

Comment 4

[Heather Arthur [:harth]]

Comment on attachment 8378720 [details] [diff] [review]
971191.patch

Review of attachment 8378720 [details] [diff] [review]:
-----------------------------------------------------------------

Running `./mach mochitest-browser browser/devtools/styleeditor`, there's one test failing. Which is good, since that means we're testing the code. We'll have to modify that test.

I think the problem illustrated in the screenshot is caused by this nodeHref issue.

::: browser/devtools/styleeditor/StyleSheetEditor.jsm
@@ +153,5 @@
>      if (!this._friendlyName) {
>        let sheetURI = this.styleSheet.href;
> +
> +      if (this.styleSheet.isOriginalSource) {
> +        this._friendlyName = CssLogic.shortSource({ href: sheetURI });

We shouldn't special case original sources, this should work the same for them.

@@ +155,5 @@
> +
> +      if (this.styleSheet.isOriginalSource) {
> +        this._friendlyName = CssLogic.shortSource({ href: sheetURI });
> +      }
> +      else if (this.styleSheet.nodeHref) {

Some stylesheets (including all original sources) won't have a nodeHref.

The old code used the document's baseURI. The old code is here:
https://hg.mozilla.org/releases/mozilla-release/file/824017eda3df/browser/devtools/styleeditor/StyleSheetEditor.jsm#l134.

The baseURI is this object: https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIURI

Unfortunately, we don't have access to the baseURI anymore with the new stylesheet actors.

According to https://developer.mozilla.org/en-US/docs/Web/API/Node.baseURI, the baseURI is usually just the window.location except in two special cases. We might have access to the url of the document from the target. Then we could convert that to an nsURI object and use the old code from there.

That's an option, another is having relative path property on each stylesheet or source on the server, which is sent with each sheet's 'form'. That's not backwards compatible.

Maybe there's another option too?

Comment 5

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

Thanks for the review Heather! I will look into this tomorrow.

Comment 6

[Patrick Brosset <:pbro>]

Inspector bug triage (filter on CLIMBING SHOES).