Closed Bug 869003 Opened 7 years ago Closed 6 years ago

Cannot inspect objects from cross-domain iframes

Categories

(DevTools :: Console, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 23

People

(Reporter: msucan, Assigned: msucan)

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

Since bug 783499 landed if you call console.log(foo) in an iframe from that is loaded from a different domain than the top level document, the |foo| object cannot be inspected. When you click the object in console's output you see a "permission denied" error in the Browser Console.
Assignee: nobody → mihai.sucan
Attached patch proposed fix (obsolete) — Splinter Review
This patch changes the web console actor in such a way that it does addDebuggee() for every new window global it encounters from console API log calls or from evals. It also changes evaluation to happen in the global window object that belongs to the object the user is working with in variables view - this avoids permission denied errors when viewing or changing properties.

The new test lives in browser/ rather than toolkit/ because the toolkit tests run in chrome privileges and I was not able to reproduce the permission denied error with cross-domain iframes.

Try push:
https://tbpl.mozilla.org/?tree=Try&rev=473c77469862
Attachment #746608 - Flags: review?(rcampbell)
Status: NEW → ASSIGNED
Depends on: 862916
Comment on attachment 746608 [details] [diff] [review]
proposed fix

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

::: toolkit/devtools/webconsole/dbg-webconsole-actors.js
@@ +269,5 @@
> +      try {
> +        global = Cu.getGlobalForObject(aValue);
> +      }
> +      catch (ex) {
> +        // The above can throw an exception if aValue is not an actual object.

should we be putting this error on the console or something? Hiding exceptions can be annoying.

@@ +793,2 @@
>        let jsObj = bindSelf.unsafeDereference();
> +      let global = global = Cu.getGlobalForObject(jsObj);

why the global = global here?

@@ +977,5 @@
>    {
>      let result = WebConsoleUtils.cloneObject(aMessage);
>      delete result.wrappedJSObject;
>  
> +    result.arguments = Array.map(aMessage.arguments || [], (aObj) => {

opportunistic arrow functioning.
Attachment #746608 - Flags: review?(rcampbell) → review+
try looks good.
(In reply to Rob Campbell [:rc] (:robcee) from comment #2)
> Comment on attachment 746608 [details] [diff] [review]
> proposed fix
> 
> Review of attachment 746608 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/devtools/webconsole/dbg-webconsole-actors.js
> @@ +269,5 @@
> > +      try {
> > +        global = Cu.getGlobalForObject(aValue);
> > +      }
> > +      catch (ex) {
> > +        // The above can throw an exception if aValue is not an actual object.
> 
> should we be putting this error on the console or something? Hiding
> exceptions can be annoying.

This is expected to be really common. I am going to add a typeof check to avoid this.


> 
> @@ +793,2 @@
> >        let jsObj = bindSelf.unsafeDereference();
> > +      let global = global = Cu.getGlobalForObject(jsObj);
> 
> why the global = global here?

Eh, editing artifact caused by several iterations of the code until I got things right.

Good catch!


> @@ +977,5 @@
> >    {
> >      let result = WebConsoleUtils.cloneObject(aMessage);
> >      delete result.wrappedJSObject;
> >  
> > +    result.arguments = Array.map(aMessage.arguments || [], (aObj) => {
> 
> opportunistic arrow functioning.

Yes. :)

Thanks for the review. Will update the patch and land it.
Whiteboard: [fixed-in-fx-team]
No longer depends on: 862916
https://hg.mozilla.org/mozilla-central/rev/2441ed727cea
Status: ASSIGNED → RESOLVED
Closed: 6 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.