Bug 182212 – Some test cases in accessibility/mac/selection-notification-focus-change.html fail

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Ryosuke Niwa]

Fix the test refactored in https://trac.webkit.org/changeset/227713.

[Radar WebKit Bug Importer]

<rdar://problem/36937147>

[Ryosuke Niwa]

Created attachment 332462 [details]
Fixes the test

[Wenson Hsieh]

Comment on attachment 332462 [details]
Fixes the test

View in context: https://bugs.webkit.org/attachment.cgi?id=332462&action=review

> Source/WebCore/ChangeLog:13
> +        (WebCore::Element::focus): Removed an unnecessarily synchronous layout update.

Nit - an unnecessary synchronous layout update.

[Ryosuke Niwa]

Comment on attachment 332462 [details]
Fixes the test

View in context: https://bugs.webkit.org/attachment.cgi?id=332462&action=review

>> Source/WebCore/ChangeLog:13
>> +        (WebCore::Element::focus): Removed an unnecessarily synchronous layout update.
> 
> Nit - an unnecessary synchronous layout update.

This is unnecessary after https://trac.webkit.org/changeset/227664 and https://trac.webkit.org/changeset/226823.

[Ryosuke Niwa]

<rdar://problem/36930430>

[EWS Watchlist]

Comment on attachment 332462 [details]
Fixes the test

Attachment 332462 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/6230620

New failing tests:
editing/input/caret-at-the-edge-of-input.html
accessibility/ios-simulator/table-cell-for-row-col.html
fast/forms/input-text-scroll-left-on-blur.html
accessibility/ios-simulator/header-elements.html

[EWS Watchlist]

Created attachment 332464 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 332462 [details]
Fixes the test

Attachment 332462 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/6231284

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 332467 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Ryosuke Niwa]

Created attachment 332560 [details]
Fixed macOS WK1 tests

[EWS Watchlist]

Comment on attachment 332560 [details]
Fixed macOS WK1 tests

Attachment 332560 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/6251492

New failing tests:
editing/input/caret-at-the-edge-of-input.html
accessibility/ios-simulator/table-cell-for-row-col.html
fast/forms/input-text-scroll-left-on-blur.html
accessibility/ios-simulator/header-elements.html

[EWS Watchlist]

Created attachment 332569 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6

[Ryosuke Niwa]

Created attachment 332853 [details]
Fixed more tests

[Ryosuke Niwa]

Created attachment 332854 [details]
Fixed more tests

[Antti Koivisto]

Comment on attachment 332854 [details]
Fixed more tests

View in context: https://bugs.webkit.org/attachment.cgi?id=332854&action=review

> Source/WebCore/editing/FrameSelection.cpp:378
> +    m_selectionRevealIntent = intent;
>      m_pendingSelectionUpdate = true;

m_selectionRevealIntent is never reset. m_pendingSelectionUpdate bit can get also set by FrameSelection::setNeedsSelectionUpdate in which case we presumably use whatever intent was set earlier. This doesn't seem right.

[Ryosuke Niwa]

(In reply to Antti Koivisto from comment #15)
> Comment on attachment 332854 [details]
> Fixed more tests
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=332854&action=review
> 
> > Source/WebCore/editing/FrameSelection.cpp:378
> > +    m_selectionRevealIntent = intent;
> >      m_pendingSelectionUpdate = true;
> 
> m_selectionRevealIntent is never reset. m_pendingSelectionUpdate bit can get
> also set by FrameSelection::setNeedsSelectionUpdate in which case we
> presumably use whatever intent was set earlier. This doesn't seem right.

That's okay. What's not okay is that we're not resetting m_selectionRevealMode in some cases. We should probably fix that in a separate patch since that's a pre-existing bug.

[Ryosuke Niwa]

Let's track that in the bug 182378.

[Ryosuke Niwa]

Created attachment 332861 [details]
Fixed per Antti's comments

[Ryosuke Niwa]

Comment on attachment 332861 [details]
Fixed per Antti's comments

Landing this manually instead.

[Ryosuke Niwa]

Committed r227983: <https://trac.webkit.org/changeset/227983>

[Ryan Haddad]

(In reply to Ryosuke Niwa from comment #20)
> Committed r227983: <https://trac.webkit.org/changeset/227983>
This change has caused LayoutTest fast/dom/adopt-node-crash-2.html to become a flaky failure on High Sierra Release WK2 and Sierra Release WK2:
https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/r228225%20(7494)/results.html

I am able to reproduce the failure with a build of r227984 with the following:
run-webkit-tests fast/dom/adopt-node-crash-2.html -fg --iter 100 --exit-after-n-failures 1

I cannot reproduce with a build of r227982.

[Ryosuke Niwa]

(In reply to Ryan Haddad from comment #21)
> (In reply to Ryosuke Niwa from comment #20)
> > Committed r227983: <https://trac.webkit.org/changeset/227983>
> This change has caused LayoutTest fast/dom/adopt-node-crash-2.html to become
> a flaky failure on High Sierra Release WK2 and Sierra Release WK2:
> https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/
> r228225%20(7494)/results.html
> 
> I am able to reproduce the failure with a build of r227984 with the
> following:
> run-webkit-tests fast/dom/adopt-node-crash-2.html -fg --iter 100
> --exit-after-n-failures 1
> 
> I cannot reproduce with a build of r227982.

I think this is a bug in the test. Tracking that in https://bugs.webkit.org/show_bug.cgi?id=182589.