Bug 154330 - Regression(r196648): window.showModalDialog is no longer undefined if the client does not allow showing modal dialog
Summary: Regression(r196648): window.showModalDialog is no longer undefined if the cli...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: WebExposed
Depends on:
Blocks: 154120
  Show dependency treegraph
 
Reported: 2016-02-16 22:09 PST by Chris Dumez
Modified: 2016-02-17 11:31 PST (History)
5 users (show)

See Also:


Attachments
Patch (11.71 KB, patch)
2016-02-16 22:18 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (11.94 KB, patch)
2016-02-16 22:24 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (12.89 KB, patch)
2016-02-17 08:57 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (12.89 KB, patch)
2016-02-17 08:58 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-02-16 22:09:28 PST
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 :(
Comment 1 Chris Dumez 2016-02-16 22:18:47 PST
Created attachment 271539 [details]
Patch
Comment 2 Chris Dumez 2016-02-16 22:24:27 PST
Created attachment 271540 [details]
Patch
Comment 3 Gavin Barraclough 2016-02-16 23:46:58 PST
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);
Comment 4 Chris Dumez 2016-02-17 08:57:01 PST
Created attachment 271558 [details]
Patch
Comment 5 Chris Dumez 2016-02-17 08:58:06 PST
Created attachment 271559 [details]
Patch
Comment 6 Chris Dumez 2016-02-17 08:58:28 PST
(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.
Comment 7 WebKit Commit Bot 2016-02-17 11:31:08 PST
Comment on attachment 271559 [details]
Patch

Clearing flags on attachment: 271559

Committed r196706: <http://trac.webkit.org/changeset/196706>
Comment 8 WebKit Commit Bot 2016-02-17 11:31:15 PST
All reviewed patches have been landed.  Closing bug.