Changeset 253799 in webkit


Ignore:
Timestamp:
Dec 19, 2019 4:28:57 PM (4 years ago)
Author:
Chris Dumez
Message:

REGRESSION: (r251677) imported/w3c/web-platform-tests/html/semantics/forms/form-submission-0/form-double-submit-3.html is a flaky failure
​https://bugs.webkit.org/show_bug.cgi?id=205164
<rdar://problem/57879042>

Reviewed by Ryosuke Niwa.

LayoutTests/imported/w3c:

Rebaseline tests that are now passing.

  • web-platform-tests/html/semantics/forms/form-submission-0/form-double-submit-3-expected.txt:
  • web-platform-tests/html/semantics/forms/form-submission-0/form-double-submit-expected.txt:

Source/WebCore:

Submitting a form should cancel any pending navigation scheduled by a previous submission of this form:

No new tests, rebaselined existing tests.

Test: fast/forms/form-double-submission.html

  • html/HTMLFormElement.cpp:

(WebCore::HTMLFormElement::submit):

  • html/HTMLFormElement.h:
  • loader/FormSubmission.h:

(WebCore::FormSubmission::cancel):
(WebCore::FormSubmission::wasCancelled const):

  • loader/FrameLoader.cpp:

(WebCore::FrameLoader::submitForm):
Drop previous non-standard compliant logic to avoid double-form submission.

  • loader/NavigationScheduler.cpp:

LayoutTests:

  • fast/forms/form-double-submission-expected.txt: Added.
  • fast/forms/form-double-submission.html: Added.
  • fast/forms/resources/form-double-submission-frame.html: Added.

Add layout test for the regression that was introduced the first time this patch landed.

  • http/tests/misc/multiple-submit-expected.txt:

Rebaseline test due to behavior change. I have verified that our new behavior on this test is
aligned with Firefox 71 and Chrome 79.

  • platform/mac/TestExpectations:

Unskip tests that are no longer flaky.

Location:
trunk
Files:
3 added
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r253795 r253799  
     12019-12-19  Chris Dumez  <cdumez@apple.com>
     2
     3        REGRESSION: (r251677) imported/w3c/web-platform-tests/html/semantics/forms/form-submission-0/form-double-submit-3.html is a flaky failure
     4        https://bugs.webkit.org/show_bug.cgi?id=205164
     5        <rdar://problem/57879042>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        * fast/forms/form-double-submission-expected.txt: Added.
     10        * fast/forms/form-double-submission.html: Added.
     11        * fast/forms/resources/form-double-submission-frame.html: Added.
     12        Add layout test for the regression that was introduced the first time this patch landed.
     13
     14        * http/tests/misc/multiple-submit-expected.txt:
     15        Rebaseline test due to behavior change. I have verified that our new behavior on this test is
     16        aligned with Firefox 71 and Chrome 79.
     17
     18        * platform/mac/TestExpectations:
     19        Unskip tests that are no longer flaky.
     20
    1212019-12-19  Chris Dumez  <cdumez@apple.com>
    222

  • trunk/LayoutTests/http/tests/misc/multiple-submit-expected.txt

    r19940 r253799  
    1 submitted=form2
     1submitted=form1

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r253797 r253799  
     12019-12-19  Chris Dumez  <cdumez@apple.com>
     2
     3        REGRESSION: (r251677) imported/w3c/web-platform-tests/html/semantics/forms/form-submission-0/form-double-submit-3.html is a flaky failure
     4        https://bugs.webkit.org/show_bug.cgi?id=205164
     5        <rdar://problem/57879042>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        Rebaseline tests that are now passing.
     10
     11        * web-platform-tests/html/semantics/forms/form-submission-0/form-double-submit-3-expected.txt:
     12        * web-platform-tests/html/semantics/forms/form-submission-0/form-double-submit-expected.txt:
     13
    1142019-12-19  Jonathan Bedard  <jbedard@apple.com>
    215

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/form-submission-0/form-double-submit-3-expected.txt

    r253666 r253799  
    55submit
    66
    7 FAIL <button> should have the same double-submit protection as <input type=submit> assert_unreached: Frame1 should not get navigated by this test. Reached unreachable code
     7PASS <button> should have the same double-submit protection as <input type=submit>
    88

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/form-submission-0/form-double-submit-expected.txt

    r253666 r253799  
    55
    66
    7 FAIL default submit action should supersede onclick submit() assert_unreached: Frame1 should not get navigated by this test. Reached unreachable code
     7PASS default submit action should supersede onclick submit()
    88

  • trunk/LayoutTests/platform/mac/TestExpectations

    r253768 r253799  
    19641964webkit.org/b/205361 [ Release ] crypto/workers/subtle/aes-indexeddb.html [ Pass Timeout ]
    19651965
    1966 webkit.org/b/205164 imported/w3c/web-platform-tests/html/semantics/forms/form-submission-0/form-double-submit-3.html [ Pass Failure ]
    1967 webkit.org/b/205164 imported/w3c/web-platform-tests/html/semantics/forms/form-submission-0/form-double-submit.html [ Pass Failure ]
    1968 
    19691966webkit.org/b/205403 accessibility/presentation-role-iframe.html [ Pass Failure ]

  • trunk/Source/WebCore/ChangeLog

    r253790 r253799  
     12019-12-19  Chris Dumez  <cdumez@apple.com>
     2
     3        REGRESSION: (r251677) imported/w3c/web-platform-tests/html/semantics/forms/form-submission-0/form-double-submit-3.html is a flaky failure
     4        https://bugs.webkit.org/show_bug.cgi?id=205164
     5        <rdar://problem/57879042>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        Submitting a form should cancel any pending navigation scheduled by a previous submission of this form:
     10        - https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#form-submission-algorithm (step 22.3)
     11
     12        No new tests, rebaselined existing tests.
     13
     14        Test: fast/forms/form-double-submission.html
     15
     16        * html/HTMLFormElement.cpp:
     17        (WebCore::HTMLFormElement::submit):
     18        * html/HTMLFormElement.h:
     19        * loader/FormSubmission.h:
     20        (WebCore::FormSubmission::cancel):
     21        (WebCore::FormSubmission::wasCancelled const):
     22
     23        * loader/FrameLoader.cpp:
     24        (WebCore::FrameLoader::submitForm):
     25        Drop previous non-standard compliant logic to avoid double-form submission.
     26
     27        * loader/NavigationScheduler.cpp:
     28
    1292019-12-19  Ryosuke Niwa  <rniwa@webkit.org>
    230

  • trunk/Source/WebCore/html/HTMLFormElement.cpp

    r253666 r253799  
    359359
    360360    auto shouldLockHistory = processingUserGesture ? LockHistory::No : LockHistory::Yes;
    361     frame->loader().submitForm(FormSubmission::create(*this, m_attributes, event, shouldLockHistory, formSubmissionTrigger));
     361    auto formSubmission = FormSubmission::create(*this, m_attributes, event, shouldLockHistory, formSubmissionTrigger);
     362    if (m_plannedFormSubmission)
     363        m_plannedFormSubmission->cancel();
     364    m_plannedFormSubmission = makeWeakPtr(formSubmission.get());
     365    frame->loader().submitForm(WTFMove(formSubmission));
    362366
    363367    if (needButtonActivation && firstSuccessfulSubmitButton)

  • trunk/Source/WebCore/html/HTMLFormElement.h

    r253666 r253799  
    182182    Vector<WeakPtr<HTMLImageElement>> m_imageElements;
    183183    WeakHashSet<HTMLFormControlElement> m_invalidAssociatedFormControls;
     184    WeakPtr<FormSubmission> m_plannedFormSubmission;
    184185
    185186    bool m_wasUserSubmitted { false };

  • trunk/Source/WebCore/loader/FormSubmission.h

    r253666 r253799  
    3434#include "FrameLoaderTypes.h"
    3535#include <wtf/URL.h>
     36#include <wtf/WeakPtr.h>
    3637
    3738namespace WebCore {
    … …  
    4142class FrameLoadRequest;
    4243
    43 class FormSubmission : public RefCounted<FormSubmission> {
     44class FormSubmission : public RefCounted<FormSubmission>, public CanMakeWeakPtr<FormSubmission> {
    4445public:
    45     enum class Method { Get, Post };
     46    enum class Method : bool { Get, Post };
    4647
    4748    class Attributes {
    … …  
    9798    void setOrigin(const String& origin) { m_origin = origin; }
    9899
     100    void cancel() { m_wasCancelled = true; }
     101    bool wasCancelled() const { return m_wasCancelled; }
     102
    99103private:
    100104    FormSubmission(Method, const URL& action, const String& target, const String& contentType, Ref<FormState>&&, Ref<FormData>&&, const String& boundary, LockHistory, Event*);
    … …  
    102106    // FIXME: Hold an instance of Attributes instead of individual members.
    103107    Method m_method;
     108    bool m_wasCancelled { false };
    104109    URL m_action;
    105110    String m_target;

  • trunk/Source/WebCore/loader/FrameLoader.cpp

    r253654 r253799  
    486486        return;
    487487
    488     // FIXME: We'd like to remove this altogether and fix the multiple form submission issue another way.
    489 
    490     // We do not want to submit more than one form from the same page, nor do we want to submit a single
    491     // form more than once. This flag prevents these from happening; not sure how other browsers prevent this.
    492     // The flag is reset in each time we start dispatching a new mouse or key down event, and
    493     // also in setView since this part may get reused for a page from the back/forward cache.
    494     // The form multi-submit logic here is only needed when we are submitting a form that affects this frame.
    495 
    496     // FIXME: Frame targeting is only one of the ways the submission could end up doing something other
    497     // than replacing this frame's content, so this check is flawed. On the other hand, the check is hardly
    498     // needed any more now that we reset m_submittedFormURL on each mouse or key down event.
    499 
    500     if (m_frame.tree().isDescendantOf(targetFrame)) {
    501         if (m_submittedFormURL == submission->requestURL())
    502             return;
     488    if (m_frame.tree().isDescendantOf(targetFrame))
    503489        m_submittedFormURL = submission->requestURL();
    504     }
    505490
    506491    submission->setReferrer(outgoingReferrer());

  • trunk/Source/WebCore/loader/NavigationScheduler.cpp

    r253666 r253799  
    278278    void fire(Frame& frame) override
    279279    {
     280        if (m_submission->wasCancelled())
     281            return;
     282
    280283        UserGestureIndicator gestureIndicator(userGestureToForward());
    281284
Note: See TracChangeset for help on using the changeset viewer.