Closed Bug 609872 Opened 14 years ago Closed 11 years ago

Ability to execute code in sub-documents (iframes/frames)

Categories

(DevTools :: Console, enhancement, P3)

enhancement

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 30

People

(Reporter: vladmaniac, Assigned: msucan)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [console-2][parity-chrome])

Attachments

(1 file, 1 obsolete file)

Build ID: Mozilla/5.0 (Windows NT 6.1; rv:2.0b8pre) Gecko/20101103 Firefox/4.0b8pre If you have any simple JS example, such as a simple alert box onclick, you can call that though the console without loading the actual page. Given that function intergrated in an iframe, it is not accessible any more from the web console. Can't do that even if moving the function to a sepparate .js file. This is not a "must", but would be very comforting to have. ========================================================================= Here is David's solution, which is simple and elegant, but I'm sure not all console users can come up with this: var iframeDoc = document.querySelectorAll("iframe")[0].contentWindow.document; then you can call: iframeDoc.fooFunction(); where fooFunction() is something you defined in your page and want to separately call from the Web Console.
Summary: Access to JS functions defined within webpages are blocked by iframes → Access to JS functions defined within webpages is blocked by iframes
I actually think that any behavior other than that which you describe would be confusing. The console would need to evaluate expressions in some sort of merged namespace, and randomly merging namespaces is likely to yield odd results. FWIW, you can shorten your workaround like this: var iframeDoc = $$('iframe')[0].contentWindow.document;
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → WONTFIX
alternatively, we could implement a "cd" helper function to change our context to the iframe's contentWindow. I believe Firebug has this.
Yeah, that's an option (though for me "cd" just seemed to send Firebug spinning). On the one hand, this seems like a bit of a niche need... but on the other, I can see someone with a lot of iframes wanting to be able to switch into one or another. I'll repurpose the bug toward that end, though this is still a feature to come sometime after Firefox 4.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Summary: Access to JS functions defined within webpages is blocked by iframes → Ability to "cd" into different contexts (like iframe documents) for JS command line
yeah, cd never worked well in my experience. Neglected feature, I think. Agree that we shouldn't hold for this bug. It's a feature request and would be nice to have.
Severity: normal → enhancement
Whiteboard: [console-2]
Priority: -- → P3
Component: Developer Tools → Developer Tools: Console
OS: Windows 7 → All
Hardware: x86 → All
Summary: Ability to "cd" into different contexts (like iframe documents) for JS command line → Ability to execute code in sub-documents (iframes/frames)
Whiteboard: [console-2] → [console-2][parity-chrome]
Chrome does have this behavior at the GUI level, and allows you to set the 'window' context, and the 'browser' context. The window context, switches between layers of iframes, while the 'browser' context allows you to switch to extension background pages, content script context, and extension page context.
doing this as defined in the 1st couple of comments is confusing because you don't know the clear scoping of variables, and secondly the scoping is mostly at the page level and not at the iframe level, so it's hard to access globals in the iframe context.
This will be done with a nice UI, not as explained in the first comments. I marked the other bug as a duplicate of this one simply for tidying up - I've been doing web console bugs triage lately.
This was probably the most requested feature in my devtools booth today. We should probably consider raising the priority on this. Doing a simple UI like Firebug's cd command would be fine by me: http://getfirebug.com/wiki/index.php/Cd
Let's remember that JSTerm is coming quite soon.
Do we have a plan for handling this in JSTerm? Could we just add a "cd" command there?
(In reply to Panos Astithas [:past] from comment #13) > Do we have a plan for handling this in JSTerm? Could we just add a "cd" > command there? That's the plan. Except that I called the command 'global' because this has nothing to do with directories.
Why "global"? "cd" is what firebug users use. I think we should re-use it. Or maybe "context".
I don't like "cd" for the obvious reason that it's not about changing directories, but it is about changing the global object.
(In reply to Joe Walker [:jwalker] from comment #16) > I don't like "cd" for the obvious reason that it's not about changing > directories, but it is about changing the global object. Apparently it worked well for Firebug users. Let's at least have an alias.
I find the fact that other browsers didn't add this quite telling. I'm fine with either though.
Chrome's solution is not terrible: https://www.evernote.com/shard/s1/sh/154250a8-92e3-4b4e-9e99-249a5cea74cc/d3e890be27f1300481705a9199addaef/deep/0/Screen%20Shot%202013-10-11%20at%2011.51.03%20AM.png I don't mind having a console helper to help navigate the frames, it's *also* great to have a visual list of frames like chrome does. Mihai - when you say this will be done with a 'nice ui', do we have design input there?
Jeff, I meant something like a drop-down selection for picking the iframe based on URL, similar to that of Chrome. A |cd| command is not easily discoverable, but I do agree we also need a console helper. On the topic of |cd| versus |global| I am of the opinion that |cd| is more appropriate - other tools use this, and there's not much benefit of using |global|.
(In reply to Mihai Sucan [:msucan] from comment #20) > Jeff, I meant something like a drop-down selection for picking the iframe > based on URL, similar to that of Chrome. A |cd| command is not easily > discoverable, but I do agree we also need a console helper. Ok, sounds good. > On the topic of |cd| versus |global| I am of the opinion that |cd| is more > appropriate - other tools use this, and there's not much benefit of using > |global|. I think I agree. When this gets landed make sure you log a docs bug.
We want iframes and any sub-docshell (window.open(…))
Blocks: 921007
why not just a select box with all the availble frames, that would be way easier than a cd command.
I know from experience that debugging for example Facebook's apps/games is just about impossible without this feature. Voted, hope it's in progress.
Status: REOPENED → NEW
Attached patch wip1 (obsolete) (deleted) — Splinter Review
Feature implementation - this is fully working. Need to write tests.
Assignee: nobody → mihai.sucan
Status: NEW → ASSIGNED
CCing Alex who was trying something similar, albeit b2g-focused, today for inspiration.
Ahah funny that we start working on that at the same time! I got something working as well, but it would be at the toolbox level. I'd easily imagine we want to end up with both. "cd" in webconsole may only switch the console scope and not the whole toolbox one.
I think "cd" should switch the console as well as scratchpad and any other JS input we come up with. Conversely, if we implement UI to do this ( chrome has a drop list for example ) the effect should be the same.
(In reply to Jeff Griffiths (:canuckistani) from comment #28) > I think "cd" should switch the console as well as scratchpad and any other > JS input we come up with. Conversely, if we implement UI to do this ( chrome > has a drop list for example ) the effect should be the same. True, but unfortunately this feature has been postponed for too long, in the hope we'll do actual work on a toolbox-level frame selection UI. We need that, but in the meantime, this is good enough. As for Scratchpad: you should be able to cd() from it as well. I'm not yet sure if cd() calls are 'shared'. I need to recheck how scratchpad uses the connection and the console actor. (In reply to Alexandre Poirot (:ochameau) from comment #27) > Ahah funny that we start working on that at the same time! > > I got something working as well, but it would be at the toolbox level. > I'd easily imagine we want to end up with both. > "cd" in webconsole may only switch the console scope and not the whole > toolbox one. This is great. You should continue the work on that, we still need a toolbox-level UI for frame selection.
Opera dragonfly had a toolbox-level UI for Frame selection (and even tab selection I think).
Attached patch bug609872-2.diff (deleted) — Splinter Review
Patch with tests. Try push: https://tbpl.mozilla.org/?tree=Try&rev=93365572d617 cd() changes the eval scope for the console actor. Scratchpad reuses the console actor, which means it gets the new eval scope (tested). Scratchpad users can use cd() as well, when they need to change the eval scope. Looking forward to your review. Thanks! (take your time with the review - I know you guys are busy in Portland!)
Attachment #8377809 - Attachment is obsolete: true
Attachment #8378401 - Flags: review?(past)
Keywords: dev-doc-needed
Comment on attachment 8378401 [details] [diff] [review] bug609872-2.diff Review of attachment 8378401 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/devtools/server/actors/webconsole.js @@ +213,5 @@ > + this._progressListenerActive = true; > + } > + }, > + > + _progressListenerActive: false, Can you clarify the purpose of this flag in a comment? Doesn't it need to be reset somewhere (maybe in _onWillNavigate)? @@ +835,5 @@ > makeDebuggeeValue: aDebuggerGlobal.makeDebuggeeValue.bind(aDebuggerGlobal), > createValueGrip: this.createValueGrip.bind(this), > sandbox: Object.create(null), > helperResult: null, > + consoleActor: this, Is this needed anywhere? It doesn't look like it is. ::: toolkit/devtools/webconsole/utils.js @@ +1515,5 @@ > + * is used to perform document.querySelector(), to find the iframe that > + * you want to cd() to. A DOMElement can be given as well, the > + * .contentWindow property is used. Lastly, you can directly pass > + * a window object. If you call cd() with no arguments, the current > + * eval scope is cleared back to its default (the top window). Firebug seems to only accept a window as a parameter, but being smarter here sounds useful and trumps strict compatibility IMO. It's not like these helpers are going to be standardized any time soon anyway.
Attachment #8378401 - Flags: review?(past) → review+
(In reply to Panos Astithas [:past] from comment #32) > Comment on attachment 8378401 [details] [diff] [review] > bug609872-2.diff > > Review of attachment 8378401 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: toolkit/devtools/server/actors/webconsole.js > @@ +213,5 @@ > > + this._progressListenerActive = true; > > + } > > + }, > > + > > + _progressListenerActive: false, > > Can you clarify the purpose of this flag in a comment? Doesn't it need to be > reset somewhere (maybe in _onWillNavigate)? I will add a comment. The progress listener doesnt need to be readded after navigation. The tab actor doesn't do it either. It continues to work. If I am not mistaken, the outer window is used. I will double-check this. > @@ +835,5 @@ > > makeDebuggeeValue: aDebuggerGlobal.makeDebuggeeValue.bind(aDebuggerGlobal), > > createValueGrip: this.createValueGrip.bind(this), > > sandbox: Object.create(null), > > helperResult: null, > > + consoleActor: this, > > Is this needed anywhere? It doesn't look like it is. Yes, check the cd() jsterm helper. > ::: toolkit/devtools/webconsole/utils.js > @@ +1515,5 @@ > > + * is used to perform document.querySelector(), to find the iframe that > > + * you want to cd() to. A DOMElement can be given as well, the > > + * .contentWindow property is used. Lastly, you can directly pass > > + * a window object. If you call cd() with no arguments, the current > > + * eval scope is cleared back to its default (the top window). > > Firebug seems to only accept a window as a parameter, but being smarter here > sounds useful and trumps strict compatibility IMO. It's not like these > helpers are going to be standardized any time soon anyway. Yep. It's nicer to do cd('iframe') than cd($('iframe').contentWindow). Thanks for the review!
Landed: https://hg.mozilla.org/integration/fx-team/rev/9be1d1bc11fc I can confirm the progress listener continues to work after page reloads. However, I updated the patch to use once(), so the event listener is cleared when it fires, and it also clears the _progressListenerActive flag. I did this to avoid potentially subtle bugs in the future when/if the behavior of that progress listener changes. Also added a comment for the flag as suggested.
Whiteboard: [console-2][parity-chrome] → [console-2][parity-chrome][fixed-in-fx-team]
Status: ASSIGNED → RESOLVED
Closed: 14 years ago11 years ago
Resolution: --- → FIXED
Whiteboard: [console-2][parity-chrome][fixed-in-fx-team] → [console-2][parity-chrome]
Target Milestone: --- → Firefox 30
Flags: needinfo?(mihai.sucan)
(In reply to Will Bamberg [:wbamberg] from comment #36) > Mihai: > https://developer.mozilla.org/en-US/docs/Tools/ > Web_Console#Working_with_iframes - does that look OK? Yes. Thank you! Much nicer explanation than what I came up with in my blog post and jsdoc. :)
Flags: needinfo?(mihai.sucan)
Keywords: verifyme
I was able to confirm the fix for this issue on Windows 7 64-bit [1], using the documentation from Comment 36 [2] with Firefox 30 Beta 5 (Build ID: 20140515140857). [1] Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 [2] https://developer.mozilla.org/en-US/docs/Tools/Web_Console#Working_with_iframes
Status: RESOLVED → VERIFIED
Keywords: verifyme
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: