Summary: | Prepare submitForm for seamless navigation | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||||||||
Component: | New Bugs | Assignee: | Adam Barth <abarth> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | dglazkov, eric, japhet, rniwa, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Adam Barth
2012-04-12 18:01:28 PDT
Created attachment 137006 [details]
Patch
Comment on attachment 137006 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137006&action=review > Source/WebCore/loader/FrameLoader.cpp:317 > + Frame* targetFrame = findFrameForNavigation(submission->target(), submission->state()->sourceDocument()); > if (!targetFrame) { > if (!DOMWindow::allowPopUp(m_frame) && !ScriptController::processingUserGesture()) > return; Can we flip the if-else below so that we can declare targetFrame inside the if? (In reply to comment #2) > (From update of attachment 137006 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=137006&action=review > > > Source/WebCore/loader/FrameLoader.cpp:317 > > + Frame* targetFrame = findFrameForNavigation(submission->target(), submission->state()->sourceDocument()); > > if (!targetFrame) { > > if (!DOMWindow::allowPopUp(m_frame) && !ScriptController::processingUserGesture()) > > return; > > Can we flip the if-else below so that we can declare targetFrame inside the if? Sure. Actually, we can't because targetFrame is used by the rest of the function (i.e., on both branches) for targetFrame->navigationScheduler()->scheduleFormSubmission(submission); (In reply to comment #4) > Actually, we can't because targetFrame is used by the rest of the function (i.e., on both branches) for targetFrame->navigationScheduler()->scheduleFormSubmission(submission); Okay. Comment on attachment 137006 [details] Patch Attachment 137006 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12396541 New failing tests: svg/text/font-size-below-point-five.svg Created attachment 137034 [details]
Archive of layout-test-results from ec2-cr-linux-01
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment on attachment 137006 [details] Patch Rejecting attachment 137006 [details] from commit-queue. New failing tests: svg/transforms/text-with-mask-with-svg-transform.svg svg/text/font-size-below-point-five.svg Full output: http://queues.webkit.org/results/12391785 Created attachment 137042 [details]
Archive of layout-test-results from ec2-cq-02
The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: ec2-cq-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment on attachment 137006 [details] Patch Rejecting attachment 137006 [details] from commit-queue. New failing tests: svg/text/font-size-below-point-five.svg Full output: http://queues.webkit.org/results/12392806 Created attachment 137053 [details]
Archive of layout-test-results from ec2-cq-01
The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: ec2-cq-01 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Ok. Maybe this is real. :) (In reply to comment #12) > Ok. Maybe this is real. :) No.... it's failing about 50% of the time on the bots. In that case, I'm just going to land this manually, but we should mark it as flaky so it doesn't annoy other folks. (In reply to comment #14) > In that case, I'm just going to land this manually, but we should mark it as flaky so it doesn't annoy other folks. Also see http://code.google.com/p/chromium/issues/detail?id=74710. Committed r114093: <http://trac.webkit.org/changeset/114093> |