Changeset 214277 in webkit


Ignore:
Timestamp:
Mar 22, 2017 2:14:23 PM (7 years ago)
Author:
Chris Dumez
Message:

WebKit should disallow beforeunload alerts from web pages users have never interacted with
​https://bugs.webkit.org/show_bug.cgi?id=169936
<rdar://problem/23798897>

Reviewed by Brent Fulgham.

LayoutTests/imported/w3c:

  • web-platform-tests/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling-expected.txt:
  • web-platform-tests/html/webappapis/scripting/events/compile-event-handler-settings-objects-expected.txt:

Rebaseline now that the CONFIRM MESSAGE lines are now longer shown. This is because there is no user interaction
with the page.

Source/WebCore:

WebKit should disallow beforeunload alerts from web pages users have never interacted with.
This reduces the risk of annoyance to the user and is allowed by the specification:

which says:
"""
The user agent is encouraged to avoid asking the user for confirmation if it judges that doing
so would be annoying, deceptive, or pointless. A simple heuristic might be that if the user
has not interacted with the document, the user agent would not ask for confirmation before
unloading it.
"""

Firefox already implements this, Chrome does not.

Tests: fast/events/beforeunload-alert-no-user-interaction.html

fast/events/beforeunload-alert-user-interaction.html
fast/events/beforeunload-alert-user-interaction2.html

  • loader/FrameLoader.cpp:

(WebCore::shouldAskForNavigationConfirmation):
(WebCore::FrameLoader::dispatchBeforeUnloadEvent):

LayoutTests:

  • fast/events/before-unload-return-string-conversion-expected.txt:
  • fast/events/before-unload-returnValue-expected.txt:

Rebaseline now that the CONFIRM MESSAGE is no longer shown. This is because there is
no user interaction with the page.

  • fast/events/beforeunload-alert-no-user-interaction-expected.txt: Added.
  • fast/events/beforeunload-alert-no-user-interaction.html: Added.
  • fast/events/beforeunload-alert-user-interaction-expected.txt: Added.
  • fast/events/beforeunload-alert-user-interaction.html: Added.
  • fast/events/beforeunload-alert-user-interaction2-expected.txt: Added.
  • fast/events/beforeunload-alert-user-interaction2.html: Added.

Add layout test coverage.

  • fast/loader/form-submission-after-beforeunload-cancel.html:
  • fast/loader/show-only-one-beforeunload-dialog.html:
  • http/tests/misc/iframe-beforeunload-dialog-matching-ancestor-securityorigin.html:
  • http/tests/misc/iframe-beforeunload-dialog-not-matching-ancestor-securityorigin.html:

Simulate user interaction with the page so that the CONFIRM MESSAGE log lines are still
shown.

Location:
trunk
Files:
6 added
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r214274 r214277  
     12017-03-22  Chris Dumez  <cdumez@apple.com>
     2
     3        WebKit should disallow beforeunload alerts from web pages users have never interacted with
     4        https://bugs.webkit.org/show_bug.cgi?id=169936
     5        <rdar://problem/23798897>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        * fast/events/before-unload-return-string-conversion-expected.txt:
     10        * fast/events/before-unload-returnValue-expected.txt:
     11        Rebaseline now that the CONFIRM MESSAGE is no longer shown. This is because there is
     12        no user interaction with the page.
     13
     14        * fast/events/beforeunload-alert-no-user-interaction-expected.txt: Added.
     15        * fast/events/beforeunload-alert-no-user-interaction.html: Added.
     16        * fast/events/beforeunload-alert-user-interaction-expected.txt: Added.
     17        * fast/events/beforeunload-alert-user-interaction.html: Added.
     18        * fast/events/beforeunload-alert-user-interaction2-expected.txt: Added.
     19        * fast/events/beforeunload-alert-user-interaction2.html: Added.
     20        Add layout test coverage.
     21
     22        * fast/loader/form-submission-after-beforeunload-cancel.html:
     23        * fast/loader/show-only-one-beforeunload-dialog.html:
     24        * http/tests/misc/iframe-beforeunload-dialog-matching-ancestor-securityorigin.html:
     25        * http/tests/misc/iframe-beforeunload-dialog-not-matching-ancestor-securityorigin.html:
     26        Simulate user interaction with the page so that the CONFIRM MESSAGE log lines are still
     27        shown.
     28
    1292017-03-22  Nan Wang  <n_wang@apple.com>
    230

  • trunk/LayoutTests/fast/events/before-unload-return-string-conversion-expected.txt

    r212625 r214277  
    1 CONFIRM NAVIGATION: PASS
    21Tests that the value returned from a beforeunload event handler gets converted to a string, even if the returnValue property was already set.
    32

  • trunk/LayoutTests/fast/events/before-unload-returnValue-expected.txt

    r155367 r214277  
    1 CONFIRM NAVIGATION: This is beforeunload from the top level frame.
    21Tests the returnValue attribute of the BeforeUnloadEvent.
    32

  • trunk/LayoutTests/fast/loader/form-submission-after-beforeunload-cancel.html

    r122687 r214277  
     1<script src="../../resources/ui-helper.js"></script>
    12<script>
    23if (window.testRunner) {
    … …  
    78
    89_confirmationDialogDisplayedOnce = false;
     10
     11function submitForm()
     12{
     13    document.forms[0].submit();
     14}
    915
    1016window.addEventListener("beforeunload", function() {
    … …  
    3541
    3642<div id="console"></div>
    37 
     43<input id="testButton" type="button" value="Click to submit form" onclick="submitForm()">
    3844<form action="resources/pass-and-notify-done.html" method="POST">
    3945</form>
    4046
    4147<script>
    42 document.forms[0].submit();
     48// Simulate a user interaction with the page so that the beforeunload alert shows.
     49const testButton = document.getElementById("testButton");
     50UIHelper.activateAt(testButton.offsetLeft + 5, testButton.offsetTop + 5);
    4351</script>

  • trunk/LayoutTests/fast/loader/show-only-one-beforeunload-dialog.html

    r152941 r214277  
     1<script src="../../resources/ui-helper.js"></script>
    12<script>
    23
    … …  
    67}
    78
     9function navigateFrame()
     10{
     11        window.location.href = 'resources/notify-done.html';
     12}
     13
    814window.onload = function()
    915{
    10         window.setTimeout("window.location.href = 'resources/notify-done.html';", 0);
     16        // Simulate a user interaction with the page so that the beforeunload alert shows.
     17        const testButton = document.getElementById("testButton");
     18        UIHelper.activateAt(testButton.offsetLeft + 5, testButton.offsetTop + 5);
    1119}
    1220
    … …  
    2129This page also has a beforeunload dialog, itself.<br>
    2230Only the dialog from this top-level frame should display, and none of the iframe ones should display.<br>
     31<input id="testButton" type="button" value="Click to navigate" onclick="navigateFrame()"><br>
    2332<iframe src="resources/iframe-with-beforeunload.html"></iframe><br>
    2433<iframe src="resources/iframe-with-beforeunload.html"></iframe><br>

  • trunk/LayoutTests/http/tests/misc/iframe-beforeunload-dialog-matching-ancestor-securityorigin.html

    r152941 r214277  
     1<script src="/js-test-resources/ui-helper.js"></script>
    12<script>
    23
    … …  
    67}
    78
     9function navigateFrame()
     10{
     11        window.location.href = 'resources/notify-done.html';
     12}
     13
    814window.onload = function()
    915{
    10         window.setTimeout("window.location.href = 'resources/notify-done.html';", 0);
     16        // Simulate a user interaction with the page so that the beforeunload alert shows.
     17        const testButton = document.getElementById("testButton");
     18        UIHelper.activateAt(testButton.offsetLeft + 5, testButton.offsetTop + 5);
    1119}
    1220
    … …  
    1523When viewed using the host 127.0.0.1, this page has an iframe whose security origin matches the main frame.<br>
    1624The iframe has a beforeunload handler, and that handler should result in the display of the page dismissal dialog.<br>
     25<input id="testButton" type="button" value="Click to navigate" onclick="navigateFrame()">
    1726<iframe src="http://127.0.0.1:8000/misc/resources/iframe-with-beforeunload.html"></iframe><br>
    1827</body>

  • trunk/LayoutTests/http/tests/misc/iframe-beforeunload-dialog-not-matching-ancestor-securityorigin.html

    r152941 r214277  
     1<script src="/js-test-resources/ui-helper.js"></script>
    12<script>
    23
    … …  
    67}
    78
     9function navigateFrame()
     10{
     11        window.location.href = 'resources/notify-done.html';
     12}
     13
    814window.onload = function()
    915{
    10         window.setTimeout("window.location.href = 'resources/notify-done.html';", 0);
     16        // Simulate a user interaction with the page so that the beforeunload alert shows.
     17        const testButton = document.getElementById("testButton");
     18        UIHelper.activateAt(testButton.offsetLeft + 5, testButton.offsetTop + 5);
    1119}
    1220
    … …  
    1523When viewed using the host 127.0.0.1, this page has an iframe whose security origin is different from the main frame.<br>
    1624The iframe has a beforeunload handler, and that handler should not result in the display of the page dismissal dialog since the security origins differ.<br>
     25<input id="testButton" type="button" value="Click to navigate" onclick="navigateFrame()"><br>
    1726<iframe src="http://localhost:8000/misc/resources/iframe-with-beforeunload.html"></iframe><br>
    1827</body>

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r214265 r214277  
     12017-03-22  Chris Dumez  <cdumez@apple.com>
     2
     3        WebKit should disallow beforeunload alerts from web pages users have never interacted with
     4        https://bugs.webkit.org/show_bug.cgi?id=169936
     5        <rdar://problem/23798897>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        * web-platform-tests/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling-expected.txt:
     10        * web-platform-tests/html/webappapis/scripting/events/compile-event-handler-settings-objects-expected.txt:
     11        Rebaseline now that the CONFIRM MESSAGE lines are now longer shown. This is because there is no user interaction
     12        with the page.
     13
    1142017-03-22  Romain Bellessort  <romain.bellessort@crf.canon.fr>
    215

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling-expected.txt

    r212625 r214277  
    1 CONFIRM NAVIGATION:
    2 CONFIRM NAVIGATION: false
    3 CONFIRM NAVIGATION: true
    4 CONFIRM NAVIGATION: 0
    5 CONFIRM NAVIGATION: foo
    6 CONFIRM NAVIGATION: foo
    7 CONFIRM NAVIGATION: foo
    8 CONFIRM NAVIGATION: foo
    9 CONFIRM NAVIGATION: foo
    10 CONFIRM NAVIGATION: foo
    111
    122PASS Returning a string must not cancel the event: CustomEvent, non-cancelable

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/webappapis/scripting/events/compile-event-handler-settings-objects-expected.txt

    r213882 r214277  
    1 CONFIRM NAVIGATION: undefined
    21
    32PASS The Function instance must be created in the Realm of the node document

  • trunk/LayoutTests/platform/ios-simulator/TestExpectations

    r214195 r214277  
    364364editing/caret/emoji.html [ Skip ]
    365365webkit.org/b/155233 fast/events/max-tabindex-focus.html [ Skip ]
     366fast/events/beforeunload-alert-user-interaction.html [ Skip ]
    366367fast/forms/validation-bubble-escape-key-dismiss.html [ Skip ]
    367368fast/forms/validation-message-maxLength.html [ Skip ]

  • trunk/Source/WebCore/ChangeLog

    r214276 r214277  
     12017-03-22  Chris Dumez  <cdumez@apple.com>
     2
     3        WebKit should disallow beforeunload alerts from web pages users have never interacted with
     4        https://bugs.webkit.org/show_bug.cgi?id=169936
     5        <rdar://problem/23798897>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        WebKit should disallow beforeunload alerts from web pages users have never interacted with.
     10        This reduces the risk of annoyance to the user and is allowed by the specification:
     11        - https://html.spec.whatwg.org/multipage/browsers.html#prompt-to-unload-a-document (Step 8):
     12        which says:
     13        """
     14        The user agent is encouraged to avoid asking the user for confirmation if it judges that doing
     15        so would be annoying, deceptive, or pointless. A simple heuristic might be that if the user
     16        has not interacted with the document, the user agent would not ask for confirmation before
     17        unloading it.
     18        """
     19
     20        Firefox already implements this, Chrome does not.
     21
     22        Tests: fast/events/beforeunload-alert-no-user-interaction.html
     23               fast/events/beforeunload-alert-user-interaction.html
     24               fast/events/beforeunload-alert-user-interaction2.html
     25
     26        * loader/FrameLoader.cpp:
     27        (WebCore::shouldAskForNavigationConfirmation):
     28        (WebCore::FrameLoader::dispatchBeforeUnloadEvent):
     29
    1302017-03-22  Brent Fulgham  <bfulgham@apple.com>
    231

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

    r214194 r214277  
    30693069}
    30703070
    3071 static bool shouldAskForNavigationConfirmation(const BeforeUnloadEvent& event)
    3072 {
     3071static bool shouldAskForNavigationConfirmation(Document& document, const BeforeUnloadEvent& event)
     3072{
     3073    bool userDidInteractWithPage = document.topDocument().lastHandledUserGestureTimestamp() > 0;
    30733074    // Web pages can request we ask for confirmation before navigating by:
    30743075    // - Cancelling the BeforeUnloadEvent (modern way)
    … …  
    30763077    // - Returning a non-empty string from the event handler, which is then set as returnValue
    30773078    //   attribute on the BeforeUnloadEvent.
    3078     return event.defaultPrevented() || !event.returnValue().isEmpty();
     3079    return userDidInteractWithPage && (event.defaultPrevented() || !event.returnValue().isEmpty());
    30793080}
    30803081
    … …  
    31033104        document->defaultEventHandler(beforeUnloadEvent.get());
    31043105
    3105     if (!shouldAskForNavigationConfirmation(beforeUnloadEvent))
     3106    if (!shouldAskForNavigationConfirmation(*document, beforeUnloadEvent))
    31063107        return true;
    31073108
Note: See TracChangeset for help on using the changeset viewer.