Bug 80205 - window.frameElement should return null if there isn't the Element for the browsing context container
Summary: window.frameElement should return null if there isn't the Element for the bro...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL: http://www.whatwg.org/specs/web-apps/...
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-02 21:06 PST by Syoichi Tsuyuhara
Modified: 2012-04-02 11:31 PDT (History)
6 users (show)

See Also:


Attachments
Patch (11.97 KB, patch)
2012-03-13 17:55 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff
Patch (12.90 KB, patch)
2012-03-13 19:53 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Syoichi Tsuyuhara 2012-03-02 21:06:48 PST
see http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.html#navigating-nested-browsing-contexts-in-the-dom .

>window . frameElement
>Returns the Element for the browsing context container.
>
>Returns null if there isn't one.
>
>Throws a SecurityError exception in cross-origin situations.

Internet Explorer 9.0.5, Firefox Nightly 13.0a1(buildID 20120302031112), Opera Next 12.00 alpha(build 1317) return null, but WebKit r109602 returns undefined, on Windows 7 Home Premium SP1 64bit.
Comment 1 Kentaro Hara 2012-03-13 17:17:04 PDT
(In reply to comment #0)
> see http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.html#navigating-nested-browsing-contexts-in-the-dom .
> 
> >window . frameElement
> >Returns the Element for the browsing context container.
> >
> >Returns null if there isn't one.
> >
> >Throws a SecurityError exception in cross-origin situations.
> 
> Internet Explorer 9.0.5, Firefox Nightly 13.0a1(buildID 20120302031112), Opera Next 12.00 alpha(build 1317) return null, but WebKit r109602 returns undefined, on Windows 7 Home Premium SP1 64bit.

Makes sense. I am making a bug-fix patch...
Comment 2 Kentaro Hara 2012-03-13 17:32:43 PDT
Maybe this is not only a problem of frameElement. contentDocument also requires null if the access is not allowed.

http://www.whatwg.org/specs/web-apps/current-work/multipage/the-iframe-element.html#dom-iframe-contentdocument

It would make sense to fix code generators to return null when shouldAllowAccessToNode() fails.
Comment 3 Kentaro Hara 2012-03-13 17:55:09 PDT
Created attachment 131764 [details]
Patch
Comment 4 Hajime Morrita 2012-03-13 19:08:52 PDT
(In reply to comment #3)
> Created an attachment (id=131764) [details]
> Patch

- Does the change covers all expectations? I wonder why there is no expected.txt for mac changed.
> Maybe this is not only a problem of frameElement. contentDocument also requires null if the access is not allowed.
Why not cover this?
Comment 5 Kentaro Hara 2012-03-13 19:53:47 PDT
Created attachment 131775 [details]
Patch
Comment 6 Kentaro Hara 2012-03-13 19:57:17 PDT
(In reply to comment #4)
> - Does the change covers all expectations? I wonder why there is no expected.txt for mac changed.
> > Maybe this is not only a problem of frameElement. contentDocument also requires null if the access is not allowed.
> Why not cover this?

- The test of frameElement is cross-frame-access-frameelement.html. I updated cross-frame-access-frameelement-expected.txt.
- Another test of frameElement is cross-frame-access-put.html. This is skipped on mac and other ports, and I just updated platform/chromium/.../cross-frame-access-put-expected.txt.
- The test of contentDocument is local-iFrame-from-remote.html. I didn't update expected.txt because there is no change in the test result.
Comment 7 WebKit Review Bot 2012-03-14 00:06:21 PDT
Comment on attachment 131775 [details]
Patch

Clearing flags on attachment: 131775

Committed r110667: <http://trac.webkit.org/changeset/110667>
Comment 8 WebKit Review Bot 2012-03-14 00:06:27 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Syoichi Tsuyuhara 2012-03-14 08:17:08 PDT
I confirmed this bug fix on WebKit r110696. Thanks, Kentaro Hara, Hajime Morrita, Adam Barth!
Comment 10 Alexey Proskuryakov 2012-04-02 10:08:20 PDT
This patch fixed both JSC and v8, but cross-platform results in http/tests/security/cross-frame-access-put-expected.txt have not been updated: bug 82751. Neither have been Qt results.

Was there a reason to keep old cross-platform results?
Comment 11 Adam Barth 2012-04-02 11:31:47 PDT
> Was there a reason to keep old cross-platform results?

Probably not.  They likely need to be updated to show the new behavior.