Changeset 227983 in webkit


Ignore:
Timestamp:
Feb 1, 2018 3:11:13 PM (6 years ago)
Author:
rniwa@webkit.org
Message:

Some test cases in accessibility/mac/selection-notification-focus-change.html fail
​https://bugs.webkit.org/show_bug.cgi?id=182212
<rdar://problem/36937147>

Reviewed by Antti Koivisto and Wenson Hsieh.

Source/WebCore:

The failure was caused by the async update of the selection appearance not preserving selection reveal intent.
Fixed the bug by storing the intent in a member variable and using it later.

  • dom/Element.cpp:

(WebCore::Element::focus): Removed an unnecessary synchronous layout update.

  • editing/FrameSelection.cpp:

(WebCore::FrameSelection::setNeedsSelectionUpdate): Use the default intent to preserve the old behavior.
(WebCore::FrameSelection::respondToNodeModification): Ditto.
(WebCore::FrameSelection::setSelection): Save the selection reveal intent.
(WebCore::FrameSelection::updateAppearanceAfterLayoutOrStyleChange): Use the saved intent.

  • editing/FrameSelection.h:
  • page/FocusController.cpp:

(WebCore::FocusController::advanceFocusDirectionally): Always update the layout before invoking
nodeRectInAbsoluteCoordinates.

LayoutTests:

Updated and rebaselined the tests.

  • accessibility/ios-simulator/header-elements.html: Force the layout after each call to element.focus

now that element.focus no longer updates the layout synchronously. Ordinarily, this will happen next time
the layout is updated for paint, by JS API, etc... but we have to force the accessibility tree to be
up-to-date for testing purposes.

  • accessibility/ios-simulator/table-cell-for-row-col.html: Ditto.
  • accessibility/mac/selection-notification-focus-change-expected.txt: Now all the test cases are passing.
  • accessibility/mac/table-with-row-col-of-headers.html: Force the layout after each call to element.focus.
  • accessibility/mac/table-with-zebra-rows.html: Ditto.
  • accessibility/scroll-to-global-point-main-window.html: Ditto.
  • accessibility/scroll-to-make-visible-with-subfocus.html: Ditto.
  • editing/input/caret-at-the-edge-of-input.html: Wait for the focused element to reveal itself by a timer.
  • fast/forms/input-text-scroll-left-on-blur.html: Ditto.
  • fast/forms/textarea-no-scroll-on-blur.html: Ditto.
  • fast/forms/textarea-scrolled-type.html: Ditto.
  • platform/mac-wk2/accessibility/mac/selection-notification-focus-change-expected.txt: Rebaselined. We now

get one less AXTextSelectionChangedFocus notification because selection updates are now coalesced as expected.

Location:
trunk
Files:
18 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r227979 r227983  
     12018-02-01  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Some test cases in accessibility/mac/selection-notification-focus-change.html fail
     4        https://bugs.webkit.org/show_bug.cgi?id=182212
     5        <rdar://problem/36937147>
     6
     7        Reviewed by Antti Koivisto and Wenson Hsieh.
     8
     9        Updated and rebaselined the tests.
     10
     11        * accessibility/ios-simulator/header-elements.html: Force the layout after each call to element.focus
     12        now that element.focus no longer updates the layout synchronously. Ordinarily, this will happen next time
     13        the layout is updated for paint, by JS API, etc... but we have to force the accessibility tree to be
     14        up-to-date for testing purposes.
     15        * accessibility/ios-simulator/table-cell-for-row-col.html: Ditto.
     16        * accessibility/mac/selection-notification-focus-change-expected.txt: Now all the test cases are passing.
     17        * accessibility/mac/table-with-row-col-of-headers.html: Force the layout after each call to element.focus.
     18        * accessibility/mac/table-with-zebra-rows.html: Ditto.
     19        * accessibility/scroll-to-global-point-main-window.html: Ditto.
     20        * accessibility/scroll-to-make-visible-with-subfocus.html: Ditto.
     21        * editing/input/caret-at-the-edge-of-input.html: Wait for the focused element to reveal itself by a timer.
     22        * fast/forms/input-text-scroll-left-on-blur.html: Ditto.
     23        * fast/forms/textarea-no-scroll-on-blur.html: Ditto.
     24        * fast/forms/textarea-scrolled-type.html: Ditto.
     25        * platform/mac-wk2/accessibility/mac/selection-notification-focus-change-expected.txt: Rebaselined. We now
     26        get one less AXTextSelectionChangedFocus notification because selection updates are now coalesced as expected.
     27
    1282018-02-01  Antoine Quint  <graouts@apple.com>
    229

  • trunk/LayoutTests/accessibility/ios-simulator/header-elements.html

    r187904 r227983  
    2626
    2727        document.getElementById("button").focus();
     28        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
    2829        var button = accessibilityController.focusedElement;
    2930
    3031        document.getElementById("header").focus();
     32        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
    3133        var header = accessibilityController.focusedElement;
    3234        shouldBeTrue("button.headerElementAtIndex(0).isEqual(header)");

  • trunk/LayoutTests/accessibility/ios-simulator/table-cell-for-row-col.html

    r187904 r227983  
    2626
    2727        document.getElementById("cell2").focus();
     28        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
    2829        var cell2 = accessibilityController.focusedElement;
    2930
    3031        document.getElementById("header").focus();
     32        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
    3133        var header = accessibilityController.focusedElement;
    3234
    … …  
    3436
    3537        document.getElementById("cell5").focus();
     38        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
    3639        var cell6 = accessibilityController.focusedElement;
    3740        shouldBeTrue("header.cellForColumnAndRow(1, 2).isEqual(cell6)");

  • trunk/LayoutTests/accessibility/mac/selection-notification-focus-change-expected.txt

    r227713 r227983  
    1919Received AXFocusChanged
    2020Received AXSelectedTextChanged
    21 FAIL userInfo["AXTextSelectionChangedFocus"] should be true (of type boolean). Was undefined (of type undefined).
     21PASS userInfo["AXTextSelectionChangedFocus"] is true
    2222Received AXSelectedTextChanged
    2323PASS userInfo["AXTextSelectionChangedFocus"] is true

  • trunk/LayoutTests/accessibility/mac/table-with-row-col-of-headers.html

    r187799 r227983  
    5959
    6060    if (window.accessibilityController) {
     61        document.getElementById("table1").focus();
     62        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
     63        shouldBe("accessibilityController.focusedElement.role", "'AXRole: AXTable'");
    6164
    62           document.getElementById("table1").focus();
    63           shouldBe("accessibilityController.focusedElement.role", "'AXRole: AXTable'");
     65        document.getElementById("table2").focus();
     66        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
     67        shouldBe("accessibilityController.focusedElement.role", "'AXRole: AXTable'");
    6468
    65           document.getElementById("table2").focus();
    66           shouldBe("accessibilityController.focusedElement.role", "'AXRole: AXTable'");
     69        document.getElementById("table3").focus();
     70        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
     71        shouldBe("accessibilityController.focusedElement.role", "'AXRole: AXGroup'");
    6772
    68           document.getElementById("table3").focus();
    69           shouldBe("accessibilityController.focusedElement.role", "'AXRole: AXGroup'");
    70 
    71           document.getElementById("table4").focus();
    72           shouldBe("accessibilityController.focusedElement.role", "'AXRole: AXGroup'");
     73        document.getElementById("table4").focus();
     74        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
     75        shouldBe("accessibilityController.focusedElement.role", "'AXRole: AXGroup'");
    7376    }
    7477

  • trunk/LayoutTests/accessibility/mac/table-with-zebra-rows.html

    r187799 r227983  
    2525
    2626    if (window.accessibilityController) {
    27 
    28           document.getElementById("table1").focus();
    29           shouldBe("accessibilityController.focusedElement.role", "'AXRole: AXTable'");
     27        document.getElementById("table1").focus();
     28        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
     29        shouldBe("accessibilityController.focusedElement.role", "'AXRole: AXTable'");
    3030    }
    3131

  • trunk/LayoutTests/accessibility/scroll-to-global-point-main-window.html

    r189149 r227983  
    2626    if (window.accessibilityController) {
    2727        target.focus();
     28        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
    2829        var targetAccessibleObject = accessibilityController.focusedElement;
    2930    }

  • trunk/LayoutTests/accessibility/scroll-to-make-visible-with-subfocus.html

    r189149 r227983  
    2121    if (window.accessibilityController) {
    2222        target.focus();
     23        internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
    2324        targetAccessibleObject = accessibilityController.focusedElement;
    2425    }

  • trunk/LayoutTests/editing/input/caret-at-the-edge-of-input.html

    r103073 r227983  
    77<script>
    88
     9if (window.testRunner)
     10    testRunner.waitUntilDone();
     11
    912var inputEdit = document.getElementById("input-edit");
    1013inputEdit.focus();
    11 var inputEditContent = inputEdit.value;
    12 inputEdit.setSelectionRange(0, 0);
    13 if (window.eventSender) {
    14     for (var i = 0; i < inputEdit.size * 1.2; ++i)
    15         eventSender.keyDown(inputEditContent[i]);
    16 }
     14setTimeout(() => {
     15    var inputEditContent = inputEdit.value;
     16    inputEdit.setSelectionRange(0, 0);
     17
     18    if (window.eventSender) {
     19        for (var i = 0; i < inputEdit.size * 1.2; ++i)
     20            eventSender.keyDown(inputEditContent[i]);
     21        testRunner.notifyDone();
     22    }
     23}, 0);
    1724
    1825</script>

  • trunk/LayoutTests/fast/forms/input-text-scroll-left-on-blur.html

    r13666 r227983  
    44<p>Tests scrolling back to the beginning when a text field blurs. The first field should be scrolled to the left, the second and third scrolled to the right.</p>
    55<script>
    6 var a = document.getElementById("a");
    7 a.focus();
    8 a.setSelectionRange(66, 66);
    9 if (window.eventSender) {
    10     eventSender.keyDown("l");
     6if (window.testRunner)
     7    testRunner.waitUntilDone();
     8
     9function wait(seconds)
     10{
     11    return new Promise((resolve) => setTimeout(resolve, seconds))
    1112}
    12 var b = document.getElementById("b");
    13 b.focus();
    14 b.setSelectionRange(66, 66);
    15 if (window.eventSender) {
    16     eventSender.keyDown("l");
     13
     14async function runTest()
     15{
     16    if (!window.eventSender) {
     17        document.body.innerHTML += 'This test requires eventSender.keyDown';
     18        return;
     19    }
     20
     21    var a = document.getElementById("a");
     22    a.focus();
     23    await wait(0);
     24    a.setSelectionRange(66, 66);
     25    if (window.eventSender)
     26        eventSender.keyDown("l");
     27
     28    var b = document.getElementById("b");
     29    b.focus();
     30    await wait(0);
     31    b.setSelectionRange(66, 66);
     32    if (window.eventSender)
     33        eventSender.keyDown("l");
     34
     35    var c = document.getElementById("c");
     36    c.focus();
     37    await wait(0);
     38    c.setSelectionRange(66, 66);
     39    if (window.eventSender)
     40        eventSender.keyDown("l");
     41
     42    if (window.testRunner)
     43        testRunner.notifyDone();
    1744}
    18 var c = document.getElementById("c");
    19 c.focus();
    20 c.setSelectionRange(66, 66);
    21 if (window.eventSender) {
    22     eventSender.keyDown("l");
    23 }
     45
     46window.onload = runTest;
     47
    2448</script>

  • trunk/LayoutTests/fast/forms/textarea-no-scroll-on-blur.html

    r121008 r227983  
    22<head>
    33<script>
    4     function test() {
    5         if (window.testRunner)
     4    function wait(seconds) {
     5        return new Promise((resolve) => setTimeout(resolve, 0), seconds);
     6    }
     7
     8    async function test() {
     9        if (window.testRunner) {
    610            testRunner.dumpAsText();
     11            testRunner.waitUntilDone();
     12        }
    713
    814        var ta = document.getElementById('ta');
    … …  
    1117        ta.focus();
    1218        ta.setSelectionRange(ta.value.length, ta.value.length);
     19        await wait(0);
    1320        ta.blur();
    1421
     22        await wait(0);
    1523        ta.focus();
     24        await wait(0);
    1625        ta.blur();
    17         if (ta.scrollTop != 0)
    18             document.getElementById('res').innerHTML = "Test Passed";
     26        await wait(0);
     27
     28        document.getElementById('res').innerHTML = ta.scrollTop != 0 ? "Test Passed" : "Test Failed";
     29
     30        if (window.testRunner)
     31            testRunner.notifyDone();
    1932    }
    2033</script>
    … …  
    30435
    3144</textarea>
    32 <div id="res">Test Failed</div>
     45<div id="res"></div>
    3346</body>
    3447</html>

  • trunk/LayoutTests/fast/forms/textarea-scrolled-type.html

    r18341 r227983  
    33<script src=../../editing/editing.js language="JavaScript" type="text/JavaScript" ></script>
    44<script>
    5 function test()
     5function wait(seconds)
    66{
     7    return new Promise((resolve) => setTimeout(resolve, 0), seconds);
     8}
     9
     10async function test()
     11{
     12    if (window.testRunner)
     13        testRunner.waitUntilDone();
     14
    715    var res = document.getElementById('res');
    816    var ta = document.getElementById('ta');
    … …  
    1018    // Send caret to bottom of textarea
    1119    ta.focus();
     20    await wait(0);
    1221    ta.setSelectionRange(ta.value.length, ta.value.length);
     22    await wait(0);
    1323    ta.blur();
     24    await wait(0);
    1425
    1526    ta.focus();
     27    await wait(0);
    1628    ta.setSelectionRange(44, 44);
    1729    typeCharacterCommand(' ');
    … …  
    2133    typeCharacterCommand('s');
    2234
     35    if (window.testRunner)
     36        testRunner.notifyDone();
    2337}
    2438</script>

  • trunk/LayoutTests/platform/mac-wk2/accessibility/mac/selection-notification-focus-change-expected.txt

    r227713 r227983  
    1919Received AXFocusChanged
    2020Received AXSelectedTextChanged
    21 FAIL userInfo["AXTextSelectionChangedFocus"] should be true (of type boolean). Was undefined (of type undefined).
    22 Received AXSelectedTextChanged
    23 FAIL userInfo["AXTextSelectionChangedFocus"] should be true (of type boolean). Was undefined (of type undefined).
     21PASS userInfo["AXTextSelectionChangedFocus"] is true
    2422PASS successfullyParsed is true
    2523

  • trunk/Source/WebCore/ChangeLog

    r227980 r227983  
     12018-02-01  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Some test cases in accessibility/mac/selection-notification-focus-change.html fail
     4        https://bugs.webkit.org/show_bug.cgi?id=182212
     5        <rdar://problem/36937147>
     6
     7        Reviewed by Antti Koivisto and Wenson Hsieh.
     8
     9        The failure was caused by the async update of the selection appearance not preserving selection reveal intent.
     10        Fixed the bug by storing the intent in a member variable and using it later.
     11
     12        * dom/Element.cpp:
     13        (WebCore::Element::focus): Removed an unnecessary synchronous layout update.
     14        * editing/FrameSelection.cpp:
     15        (WebCore::FrameSelection::setNeedsSelectionUpdate): Use the default intent to preserve the old behavior.
     16        (WebCore::FrameSelection::respondToNodeModification): Ditto.
     17        (WebCore::FrameSelection::setSelection): Save the selection reveal intent.
     18        (WebCore::FrameSelection::updateAppearanceAfterLayoutOrStyleChange): Use the saved intent.
     19        * editing/FrameSelection.h:
     20        * page/FocusController.cpp:
     21        (WebCore::FocusController::advanceFocusDirectionally): Always update the layout before invoking
     22        nodeRectInAbsoluteCoordinates.
     23
    1242018-02-01  Zalan Bujtas  <zalan@apple.com>
    225

  • trunk/Source/WebCore/dom/Element.cpp

    r227714 r227983  
    24242424    }
    24252425
    2426     // Setting the focused node above might have invalidated the layout due to scripts.
    2427     document().updateLayoutIgnorePendingStylesheets();
    2428 
    24292426    SelectionRevealMode revealMode = SelectionRevealMode::Reveal;
    24302427#if PLATFORM(IOS)

  • trunk/Source/WebCore/editing/FrameSelection.cpp

    r227092 r227983  
    375375    m_alwaysAlignCursorOnScrollWhenRevealingSelection = align == AlignCursorOnScrollAlways;
    376376
     377    m_selectionRevealIntent = intent;
    377378    m_pendingSelectionUpdate = true;
    378379
    … …  
    403404void FrameSelection::setNeedsSelectionUpdate()
    404405{
     406    m_selectionRevealIntent = AXTextStateChangeIntent();
    405407    m_pendingSelectionUpdate = true;
    406408    if (RenderView* view = m_frame->contentRenderer())
    … …  
    527529
    528530            // Trigger a selection update so the selection will be set again.
     531            m_selectionRevealIntent = AXTextStateChangeIntent();
    529532            m_pendingSelectionUpdate = true;
    530533            renderView->frameView().scheduleSelectionUpdate();
    … …  
    24482451
    24492452    setCaretRectNeedsUpdate();
    2450     updateAndRevealSelection(AXTextStateChangeIntent());
     2453    updateAndRevealSelection(m_selectionRevealIntent);
    24512454    updateDataDetectorsForSelection();
    24522455}

  • trunk/Source/WebCore/editing/FrameSelection.h

    r227092 r227983  
    348348
    349349    SelectionRevealMode m_selectionRevealMode { SelectionRevealMode::DoNotReveal };
     350    AXTextStateChangeIntent m_selectionRevealIntent;
    350351
    351352    bool m_caretInsidePositionFixed : 1;

  • trunk/Source/WebCore/page/FocusController.cpp

    r224320 r227983  
    10821082    Node* container = focusedDocument;
    10831083
    1084     if (is<Document>(*container))
    1085         downcast<Document>(*container).updateLayoutIgnorePendingStylesheets();
     1084    focusedDocument->updateLayoutIgnorePendingStylesheets();
    10861085
    10871086    // Figure out the starting rect.
    … …  
    11041103    do {
    11051104        consumed = advanceFocusDirectionallyInContainer(container, startingRect, direction, event);
     1105        focusedDocument->updateLayoutIgnorePendingStylesheets();
    11061106        startingRect = nodeRectInAbsoluteCoordinates(container, true /* ignore border */);
    11071107        container = scrollableEnclosingBoxOrParentFrameForNodeInDirection(direction, container);
    1108         if (is<Document>(container))
    1109             downcast<Document>(*container).updateLayoutIgnorePendingStylesheets();
    11101108    } while (!consumed && container);
    11111109
Note: See TracChangeset for help on using the changeset viewer.