Bug 162243

Summary: Suppress JavaScript prompts early on in certain cases
Product: WebKit Reporter: Anders Carlsson <andersca>
Component: New BugsAssignee: Anders Carlsson <andersca>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cdumez, commit-queue, dbates, japhet
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=165032
Attachments:
Description Flags
Patch
none
Patch none

Anders Carlsson
Reported 2016-09-19 16:17:40 PDT
Suppress JavaScript prompts early on in certain cases
Attachments
Patch (2.66 KB, patch)
2016-09-19 16:18 PDT, Anders Carlsson 		no flags
DetailsFormatted DiffDiff
Patch (4.55 KB, patch)
2016-09-19 16:29 PDT, Anders Carlsson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Anders Carlsson
Comment 1 2016-09-19 16:18:37 PDT
Created attachment 289271 [details] Patch
Geoffrey Garen
Comment 2 2016-09-19 16:26:58 PDT
Comment on attachment 289271 [details] Patch What about WK1? Are we sure that window.open() on a timer can't interrupt a navigation in a non-window.open document? Need some export action: "WebCore::FrameLoaderStateMachine::isDisplayingInitialEmptyDocument() const", referenced from: WebKit::shouldSuppressJavaScriptDialogs(WebCore::Frame&) in WebChromeClient.o r=me
Anders Carlsson
Comment 3 2016-09-19 16:29:19 PDT
Created attachment 289278 [details] Patch
Geoffrey Garen
Comment 4 2016-09-19 16:36:38 PDT
Comment on attachment 289278 [details] Patch r=me
Chris Dumez
Comment 5 2016-09-19 16:36:53 PDT
Comment on attachment 289278 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=289278&action=review > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:382 > +static bool shouldSuppressJavaScriptDialogs(Frame& frame) Why are we doing this in WebKit2 and not on DOMWindow in WebCore? Note that we already have page->arePromptsAllowed() for similar purposes.
Anders Carlsson
Comment 6 2016-09-19 16:39:24 PDT
(In reply to comment #5) > Comment on attachment 289278 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=289278&action=review > > > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:382 > > +static bool shouldSuppressJavaScriptDialogs(Frame& frame) > > Why are we doing this in WebKit2 and not on DOMWindow in WebCore? Note that > we already have page->arePromptsAllowed() for similar purposes. It's a targeted fix for a WK2 specific problem.
Chris Dumez
Comment 7 2016-09-19 16:39:35 PDT
Comment on attachment 289278 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=289278&action=review >> Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:382 >> +static bool shouldSuppressJavaScriptDialogs(Frame& frame) > > Why are we doing this in WebKit2 and not on DOMWindow in WebCore? Note that we already have page->arePromptsAllowed() for similar purposes. See ForbidPromptsScope in FrameLoader.
Anders Carlsson
Comment 8 2016-09-19 16:41:51 PDT
(In reply to comment #7) > Comment on attachment 289278 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=289278&action=review > > >> Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:382 > >> +static bool shouldSuppressJavaScriptDialogs(Frame& frame) > > > > Why are we doing this in WebKit2 and not on DOMWindow in WebCore? Note that we already have page->arePromptsAllowed() for similar purposes. > > See ForbidPromptsScope in FrameLoader. Using ForbidPromptsScope in this case isn't possible (it certainly won't make things easier).
Chris Dumez
Comment 9 2016-09-19 16:47:16 PDT
(In reply to comment #8) > (In reply to comment #7) > > Comment on attachment 289278 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=289278&action=review > > > > >> Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:382 > > >> +static bool shouldSuppressJavaScriptDialogs(Frame& frame) > > > > > > Why are we doing this in WebKit2 and not on DOMWindow in WebCore? Note that we already have page->arePromptsAllowed() for similar purposes. > > > > See ForbidPromptsScope in FrameLoader. > > Using ForbidPromptsScope in this case isn't possible (it certainly won't > make things easier). Ok, just making sure this was intentional.
Anders Carlsson
Comment 10 2016-09-19 16:49:13 PDT
Committed r206132: <http://trac.webkit.org/changeset/206132>
Alex Christensen
Comment 11 2016-09-19 21:33:25 PDT
Should this still be r?
Note You need to log in before you can comment on or make changes to this bug.