Closed Bug 740541 Opened 8 years ago Closed 7 years ago

Style sheets in iframes are not listed in the style editor

Categories

(DevTools :: Style Editor, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 23

People

(Reporter: dangoor, Assigned: dcrewi)

Details

Attachments

(1 file, 1 obsolete file)

STR:

1. Go to any site with iframes
   here's a simple example: http://manda.com/iframe/
2. open the style editor
3. only stylesheets from the main page are listed

expected results: should be able to edit styles that live within the iframes


This is also noticeable if you inspect an element in the iframe and then click the link from the property view into the style editor. The editor that pops up is empty.
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
Assignee: nobody → dcrewi
Attached patch patch v1 (obsolete) — Splinter Review
Attachment #739155 - Flags: review?(jwalker)
Comment on attachment 739155 [details] [diff] [review]
patch v1

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

I'm going to bounce this to Heather who is working on the Style Editor right now.
Attachment #739155 - Flags: review?(jwalker) → review?(harthur)
Attachment #739155 - Flags: review?(harthur) → review?(fayearthur)
Status: NEW → ASSIGNED
Thanks for the patch, David. This might have to wait for bug 816967 (remote style editor). That patch changes the structure of the style editor a lot, and should land very soon.
Comment on attachment 739155 [details] [diff] [review]
patch v1

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

So sorry about the delay. This looks good. It'll have to be rebased on the new Style Editor that uses the remote debugging protocol. If you want to do that, go for it. There is no more StyleEditorChrome.jsm file, this code would go in the StyleEditorActor in toolkit/devtools/styleeditor/dbg-styleeditor-actors.js, and the test would have to use the new API (see the other tests for reference).

Otherwise, I can rebase it myself soon, it's up to you. Thanks again for the patch.

::: browser/devtools/styleeditor/StyleEditorChrome.jsm
@@ +367,5 @@
> +        this._showImportedStyleSheets(styleSheet);
> +      }
> +      let iframes = document.getElementsByTagName("iframe");
> +      for (let i = 0; i < iframes.length; ++i) {
> +        let iframe = iframes[i];

Don't have to change this, but cool fact, we can use for...of loops here to avoid the counter. `for (let iframe of iframes)`.
It wasn't a rebase so much as a rewrite. At least the test didn't have to be modified much.
Attachment #739155 - Attachment is obsolete: true
Attachment #739155 - Flags: review?(fayearthur)
Attachment #745934 - Flags: review?(fayearthur)
Comment on attachment 745934 [details] [diff] [review]
patch v2, rewrite on top of remotable style editor

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

Thanks for rewriting it for the remote editor. This looks good. I've got a try server run here: https://tbpl.mozilla.org/?tree=Try&rev=a5d226c406b9. Barring any failures I can check it in.

::: browser/devtools/styleeditor/test/browser_styleeditor_bug_740541_iframes.js
@@ +71,5 @@
> +
> +  waitForExplicitFinish();
> +  let styleSheetCount = 0;
> +  addTabAndOpenStyleEditor(function (aPanel) {
> +    aPanel._debuggee.on("stylesheet-added", function () {

Should wait for the aPanel.UI.on("editor-added") event since we're checking editors.

I can change this before checking in.
Attachment #745934 - Flags: review?(fayearthur) → review+
https://hg.mozilla.org/mozilla-central/rev/3163ebad7884
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 23
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.