969900 - Saving an inline stylesheet causes all inline stylesheets to get the same name after page is reloaded

Status: VERIFIED FIXED

(DevTools :: Style Editor, defect)

Description

[Sami Jaktholm]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140208201718

Steps to reproduce:

1. Copy and paste following to the address bar:
	data:text/html;utf-8,<style>div {margin:0}</style><style>div {margin:1}</style>
2. Open style editor
3. Save the first inline style sheet to a file called 'first'.
4. Reload the page



Actual results:

After reloading all inline style sheets have the same name 'first' instead of '<inline style sheet #n>' in the list of style sheets.


Expected results:

Only the first one should have the name 'first', the second should be '<inline stylesheet #1>'.

This is a regression caused by revision 167232:2ca1cd74ca39. The culprit here seems to be that the saved files are indexed by the href of the style sheet and inline style sheets don't have one. Thus all inline style sheets get the same file and name after the page is reloaded.

Comment 1

[Sami Jaktholm]

Attachment: An image of this bug at google.com

Comment 2

[Sami Jaktholm]

Attachment: styleeditor-fix-names-of-inline-stylesheets-after-save.patch

A patch that identifies inline style sheets by their
1) host page and
2) index at that page.

That should be enough for most cases to identify style sheets across page reloads. The bulletproof method would be a checksum of the contents but that sounds a bit harsh for a simple feature like this...

Comment 3

[Heather Arthur [:harth]]

Comment on attachment 8372987 [details] [diff] [review]
styleeditor-fix-names-of-inline-stylesheets-after-save.patch

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

This is a fantastic patch, thank you. Just one small note, I can just fix this before I check it in.

Thanks Sami.

::: browser/devtools/styleeditor/test/browser_styleeditor_inline_friendly_names.js
@@ +13,5 @@
> +{
> +  waitForExplicitFinish();
> +
> +  addTabAndOpenStyleEditor(function(panel) {
> +    gContentWin = gBrowser.selectedTab.linkedBrowser.contentWindow.wrappedJSObject;

you can just use `content.window` here

@@ +96,5 @@
> +  gUI = null;
> +  gContentWin = null;
> +
> +  addTabAndOpenStyleEditor(function(panel) {
> +    gContentWin = gBrowser.selectedTab.linkedBrowser.contentWindow.wrappedJSObject;

same here, `content.window`

Comment 4

[Heather Arthur [:harth]]

try: https://tbpl.mozilla.org/?tree=Try&rev=bbc523827b02

Comment 5

[Heather Arthur [:harth]]

Attachment: Patch to check in

Patch for check in, carrying over review.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/c9d4672c0205

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

[Paul Silaghi, QA [:pauly]]

Reproduced in Nightly 2014-02-08.
Verified fixed 30.0a1 (2014-03-10), win 7 x64