613013 - Console API fails to work with iframes

Status: RESOLVED DUPLICATE of bug 609950

(DevTools :: General, defect)

Description

[Mihai Sucan [:msucan]]

Since the lazy console patch landed the window.console API fails to work when you have an iframe in the same page.

Comment 1

[Mihai Sucan [:msucan]]

Attachment: test case

Comment 2

[David Dahl :ddahl]

Comment on attachment 491299 [details] [diff] [review]
test case

This WFM. I opened this file, opened the console, and clicked the button. I saw the message from the script in console. The console continued to work.

Now, I did try to call the console from the iframe window, which did not work:

 $$("iframe")[0].contentWindow.console.log("foo")

Can you clarify your STR?

Comment 3

[David Dahl :ddahl]

Attachment: another test case

Here is another, static test case. a document with an iframe doc. They both attempt to log to the console. only the iframe console.log works

Comment 4

[Mihai Sucan [:msucan]]

STR for attachment 491299 [details] [diff] [review]:

1. open the TC.
2. open the Web Console.
3. reload.
4. click button.

Expected result: a console.log() message is shown in the Web Console.

Actual result: no message is shown.

David: it works when you first open the Web Console. It fails after reload.

Comment 5

[Mihai Sucan [:msucan]]

Attachment: proposed fix

Proposed fix.

Explanation of the issue:

- The windowInitializer() method is invoked for each global window object created in a web page. This in turn calls registerDisplay() for each such global window object. registerDisplay() does:

    var windowId = this.getWindowId(aContentWindow);
    this._headsUpDisplays[aHUDId] = { id: aHUDId, windowId: windowId };

which maps hudIds to windowIds. You immediately notice that when an iframe is loaded the windowId of the iframe takes over the windowId of the top window.

- The ConsoleAPIObserver does:

  let win = HUDService.getWindowByWindowId(windowId).top;

(where windowId comes from the Console API service)

the window object is not really used, but I presume it's checked only to see if the window is not obsolete by the time the HUDService code executes.

... then the ConsoleAPIObserver loops through HUDService._headsUpDisplays to find the hudId that maps to windowId. This is where it fails: the console.log() message comes from the top page window, and the hudId is only mapped to the iframe windowId.

The patch attached here does:

- fixes the ConsoleAPIObserver such that it retrieves the hudId based on the window object itself. We have the windowRegistry. This alone fixes the issue.

- fixes the registerDisplay() method to only store the top windowId in the _headsUpDisplays object. Again, this change alone would fix the issue reported in this bug.

Comments are welcome!

Comment 6

[Mihai Sucan [:msucan]]

Asking for blocking2.0+: an important Web Console functionality fails to work in a very common use-case: pages that have iframes cannot always use the console API.

Thanks!

Comment 7

[Mihai Sucan [:msucan]]

Comment on attachment 491331 [details]
another test case

The TC has a bug. testcase.html has <script> as fallback content for the <iframe>. That's one reason why the parent console.log() message fails to show.

If you change to:
    <iframe src="iframe.html"></iframe>
... it works.

However, this TC tests the same thing as the testcase I provided. This means it fails on the default branch build. It works with the proposed fix.

Comment 8

[David Dahl :ddahl]

(In reply to comment #7)

> However, this TC tests the same thing as the testcase I provided. This means it
> fails on the default branch build. It works with the proposed fix.

Indeed - but this is much simpler and straight forward. It is good practice to provide the simplest test case possible.

Comment 9

[David Dahl :ddahl]

Comment on attachment 491490 [details] [diff] [review]
proposed fix

nice work.

Comment 10

[Mihai Sucan [:msucan]]

Comment on attachment 491490 [details] [diff] [review]
proposed fix

David, thanks for your feedback+! Asking for review now.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Going to dupe this to bug 609950 and roll some of the changes into that.