Bug 181416 – We should not return undefined for most properties of a detached Window

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Chris Dumez]

We should not return undefined for most properties on a detached Window. WebKit currently only exposes "closed" and "close" properties on detached / frameless windows. However, this does not match the HTML specification [1] or the behavior of Firefox and Chrome.

[Chris Dumez]

<rdar://problem/36162489>

[Chris Dumez]

Created attachment 330756 [details]
Patch

[Chris Dumez]

Comment on attachment 330756 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=330756&action=review

> LayoutTests/fast/frames/detached-frame-property.html:37
> +    // Both Chrome and Firefox disagree with us and return a valid object.

I'll look into this in a follow-up.

[Ryosuke Niwa]

Comment on attachment 330756 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=330756&action=review

> LayoutTests/fast/frames/detached-frame-property-expected.txt:16
> +PASS !detachedWindow.navigator is true

How about detachedWindow.location?
Is it null? If it's not null, what happens when you try to navigate?

How about other methods like alert, confirm, etc...?

[Chris Dumez]

Comment on attachment 330756 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=330756&action=review

>> LayoutTests/fast/frames/detached-frame-property-expected.txt:16
>> +PASS !detachedWindow.navigator is true
> 
> How about detachedWindow.location?
> Is it null? If it's not null, what happens when you try to navigate?
> 
> How about other methods like alert, confirm, etc...?

Location is tested below and is null. This is not as per spec and does not match other browsers (which keep returning the location object). I will look into returning a valid object for location in a follow-up as I already commented below.

Note that we do not allow anything new here, you could already store frame.contentWindow or frame.contentWindow.location or (or other window properties) in a local variable, then detach the frame, then use that local variable. Therefore, we already deal with these objects being used without a frame.

In the case of window.alert / confirm, they return early when m_frame is null. As I said, we already handle this fine because JS could already call these operations on a frameless window by simply storing the operation in a local variable, then detached the frame and call the operation on the frameless window.

[Chris Dumez]

Comment on attachment 330756 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=330756&action=review

>>> LayoutTests/fast/frames/detached-frame-property-expected.txt:16
>>> +PASS !detachedWindow.navigator is true
>> 
>> How about detachedWindow.location?
>> Is it null? If it's not null, what happens when you try to navigate?
>> 
>> How about other methods like alert, confirm, etc...?
> 
> Location is tested below and is null. This is not as per spec and does not match other browsers (which keep returning the location object). I will look into returning a valid object for location in a follow-up as I already commented below.
> 
> Note that we do not allow anything new here, you could already store frame.contentWindow or frame.contentWindow.location or (or other window properties) in a local variable, then detach the frame, then use that local variable. Therefore, we already deal with these objects being used without a frame.
> 
> In the case of window.alert / confirm, they return early when m_frame is null. As I said, we already handle this fine because JS could already call these operations on a frameless window by simply storing the operation in a local variable, then detached the frame and call the operation on the frameless window.

E.g. window.alert on a detached window was already easy to achieve via:
window.alert.call(aDetachedWindow, "test")

[Ryosuke Niwa]

Comment on attachment 330756 [details]
Patch

Sounds good to me.

[WebKit Commit Bot]

Comment on attachment 330756 [details]
Patch

Clearing flags on attachment: 330756

Committed r226676: <https://trac.webkit.org/changeset/226676>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.