Bug 154330 – Regression(r196648): window.showModalDialog is no longer undefined if the client does not allow showing modal dialog

• Component: Bindings
• Status: RESOLVED FIXED

[Chris Dumez]

window.showModalDialog is no longer undefined if the client does not allow showing modal dialog after r196648.
We did not have test coverage for this :(

[Chris Dumez]

Created attachment 271539 [details]
Patch

[Chris Dumez]

Created attachment 271540 [details]
Patch

[Gavin Barraclough]

Comment on attachment 271540 [details]
Patch

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

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:288
> +        }

I think there is a bug here that this will do the wrong thing if your DOM contains an item named "showModalDialog" – the named getter will fail t pick it up.

I think you can easily fix by replacing:

> if (!DOMWindow::canShowModalDialog(frame)) {
>     slot.setUndefined();
>     return false;
> }

With:

> if (!DOMWindow::canShowModalDialog(frame))
>     return jsDOMWindowGetOwnPropertySlotNamedItemGetter(thisObject, *frame, exec, propertyName, slot);

[Chris Dumez]

Created attachment 271558 [details]
Patch

[Chris Dumez]

Created attachment 271559 [details]
Patch

[Chris Dumez]

(In reply to comment #3)
> Comment on attachment 271540 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=271540&action=review
> 
> > Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:288
> > +        }
> 
> I think there is a bug here that this will do the wrong thing if your DOM
> contains an item named "showModalDialog" – the named getter will fail t pick
> it up.
> 
> I think you can easily fix by replacing:
> 
> > if (!DOMWindow::canShowModalDialog(frame)) {
> >     slot.setUndefined();
> >     return false;
> > }
> 
> With:
> 
> > if (!DOMWindow::canShowModalDialog(frame))
> >     return jsDOMWindowGetOwnPropertySlotNamedItemGetter(thisObject, *frame, exec, propertyName, slot);

Fixed and added test coverage for this.

[WebKit Commit Bot]

Comment on attachment 271559 [details]
Patch

Clearing flags on attachment: 271559

Committed r196706: <http://trac.webkit.org/changeset/196706>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.