Bug 203264 – [iOS] 3 editing/pasteboard/smart-paste-paragraph-* tests are flaky

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Russell Epstein]

The following three layout tests have been flaky on iOS since they landed in r243124:

editing/pasteboard/smart-paste-paragraph-001.html
editing/pasteboard/smart-paste-paragraph-002.html
editing/pasteboard/smart-paste-paragraph-004.html

Test History:

https://results.webkit.org/?suite=layout-tests&suite=layout-tests&suite=layout-tests&test=editing%2Fpasteboard%2Fsmart-paste-paragraph-001.html&test=editing%2Fpasteboard%2Fsmart-paste-paragraph-002.html&test=editing%2Fpasteboard%2Fsmart-paste-paragraph-004.html&platform=ios


Steps to Reproduce:

run-webkit-tests --ios-simulator --no-retry --no-sample-on-timeout -f editing/pasteboard/smart-paste-paragraph-001.html editing/pasteboard/smart-paste-paragraph-002.html editing/pasteboard/smart-paste-paragraph-004.html --iter 1000


Diffs:


editing/pasteboard/smart-paste-paragraph-001.html
Diff:

--- /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/editing/pasteboard/smart-paste-paragraph-001-expected.txt
+++ /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/editing/pasteboard/smart-paste-paragraph-001-actual.txt
@@ -7,10 +7,6 @@
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 15 of #text > DIV > DIV > BODY > HTML > #document to 15 of #text > DIV > DIV > BODY > HTML > #document toDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 Tests: 
 Smart paste when pasting a paragraph between two paragraphs.
 Expected Results: 
@@ -22,8 +18,6 @@
 Last test paragraph.
 Test paragraph.
 
-Test paragraph.
-
 Last test paragraph.
 execCopyCommand: <div id="test"> Test paragraph. </div> <div><br></div> <div> Last test paragraph. </div>
-execPasteCommand: <div id="test"> Test paragraph. </div> <div><br></div><div>Test paragraph.<br></div><div><br></div> <div> Last test paragraph. </div>
+execPasteCommand: <div id="test"> Test paragraph. </div> <div><br></div> <div> Last test paragraph. </div>

editing/pasteboard/smart-paste-paragraph-002.html
Diff:

--- /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/editing/pasteboard/smart-paste-paragraph-002-expected.txt
+++ /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/editing/pasteboard/smart-paste-paragraph-002-actual.txt
@@ -7,10 +7,6 @@
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 12 of #text > PRE > BODY > HTML > #document to 12 of #text > PRE > BODY > HTML > #document givenAction:WebViewInsertActionPasted
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 3 of PRE > BODY > HTML > #document to 3 of PRE > BODY > HTML > #document toDOMRange:range from 0 of PRE > PRE > BODY > HTML > #document to 0 of PRE > PRE > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 Tests: 
 Smart paste when pasting a paragraph between two paragraphs in a pre element.
 Expected Results: 
@@ -22,8 +18,6 @@
 Last test paragraph.
 Test paragraph.
 
-Test paragraph.
-
 Last test paragraph.
 execCopyCommand: <span id="test">Test</span> paragraph. <br>Last test paragraph.
-execPasteCommand: <span id="test">Test</span> paragraph. <pre id="root" class="editing" style="word-wrap: break-word;"><span><br></span></pre><pre id="root" class="editing" style="word-wrap: break-word;"><span id="test">Test</span> paragraph.</pre><pre id="root" class="editing" style="word-wrap: break-word;"><br></pre>Last test paragraph.
+execPasteCommand: <span id="test">Test</span> paragraph. <br>Last test paragraph.

editing/pasteboard/smart-paste-paragraph-004.html
Diff:

--- /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/editing/pasteboard/smart-paste-paragraph-004-expected.txt
+++ /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/editing/pasteboard/smart-paste-paragraph-004-actual.txt
@@ -7,10 +7,6 @@
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 15 of #text > DIV > DIV > BODY > HTML > #document to 15 of #text > DIV > DIV > BODY > HTML > #document toDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 15 of #text > DIV > DIV > BODY > HTML > #document to 15 of #text > DIV > DIV > BODY > HTML > #document toDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -30,9 +26,7 @@
 
 Test paragraph.
 
-Test paragraph.
-
 Last test paragraph.
 execCopyCommand: <div id="test"> Test paragraph. </div> <div><br></div> <div> Last test paragraph. </div>
+execPasteCommand: <div id="test"> Test paragraph. </div> <div><br></div> <div> Last test paragraph. </div>
 execPasteCommand: <div id="test"> Test paragraph. </div> <div><br></div><div>Test paragraph.<br></div><div><br></div> <div> Last test paragraph. </div>
-execPasteCommand: <div id="test"> Test paragraph. </div> <div><br></div><div>Test paragraph.<br></div><div><br></div><div>Test paragraph.<br></div><div><br></div> <div> Last test paragraph. </div>

[Radar WebKit Bug Importer]

<rdar://problem/56512107>

[Russell Epstein]

Issue appears to be limited to Release builds.

Updated TestExpectations in r251449

[Aakash Jain]

@Russell, Can you please check the fourth test as well: editing/pasteboard/smart-paste-paragraph-003.html

This is also seems extremely flaky and is slowing down EWS (ios-wk2 queue). e.g.: https://ews-build.webkit.org/#/builders/24/builds/2333
https://ews-build.webkit.org/#/builders/24/builds/2336
https://ews-build.webkit.org/#/builders/24/builds/2341

[Truitt Savell]

Strange(In reply to Aakash Jain from comment #3)
> @Russell, Can you please check the fourth test as well:
> editing/pasteboard/smart-paste-paragraph-003.html
> 
> This is also seems extremely flaky and is slowing down EWS (ios-wk2 queue).
> e.g.: https://ews-build.webkit.org/#/builders/24/builds/2333
> https://ews-build.webkit.org/#/builders/24/builds/2336
> https://ews-build.webkit.org/#/builders/24/builds/2341

This is strange because we are not seeing that test at all on the testers. This may be a test list related to only the EWS

[Aakash Jain]

(In reply to Truitt Savell from comment #4)
> This is strange because we are not seeing that test at all on the testers.
> This may be a test list related to only the EWS
This is clearly failing on EWS consistently, we need to do something about this test. editing/pasteboard/smart-paste-paragraph-003.html

e.g.: https://ews-build.webkit.org/#/builders/24/builds/2652

[Jonathan Bedard]

(In reply to Aakash Jain from comment #5)
> (In reply to Truitt Savell from comment #4)
> > This is strange because we are not seeing that test at all on the testers.
> > This may be a test list related to only the EWS
> This is clearly failing on EWS consistently, we need to do something about
> this test. editing/pasteboard/smart-paste-paragraph-003.html
> 
> e.g.: https://ews-build.webkit.org/#/builders/24/builds/2652

I saw an example on GuardMalloc internally.

[Truitt Savell]

I was able to reproduce this test failure locally by just running the test by itself. It fails every time. I thought it would require a test list

[Truitt Savell]

So this is occurring on EWS becasue:

on regular testers the sharding of tests is like this:
editing/pasteboard/smart-paste-008.html
editing/pasteboard/smart-paste-in-text-control.html
editing/pasteboard/smart-paste-paragraph-001.html
editing/pasteboard/smart-paste-paragraph-002.html
editing/pasteboard/smart-paste-paragraph-003.html
editing/pasteboard/smart-paste-paragraph-004.html
editing/pasteboard/style-from-rules.html
editing/pasteboard/styled-element-markup.html

while on EWS the sharding is like this:
editing/pasteboard/smart-paste-006.html
editing/pasteboard/smart-paste-007.html
editing/pasteboard/smart-paste-008.html
editing/pasteboard/smart-paste-in-text-control.html
editing/pasteboard/smart-paste-paragraph-003.html
editing/pasteboard/style-from-rules.html
editing/pasteboard/styled-element-markup.html

because editing/pasteboard/smart-paste-paragraph-003.html is running without these other three tests it is failing. some state is not being reset.

[Truitt Savell]

editing/pasteboard/smart-paste-paragraph-003.html fails even on a checkout of r246110 which I chose arbitrarily. so this is probably not a new failure but the test order must have changed recently

[Truitt Savell]

marked test as failing in https://trac.webkit.org/changeset/251609/webkit to remedy this for now

[Alexey Proskuryakov]

This makes me wonder if this has the same root cause as the bug Wenson just fixed, with software keyboard appearing unexpectedly.

[Wenson Hsieh]

(In reply to Alexey Proskuryakov from comment #11)
> This makes me wonder if this has the same root cause as the bug Wenson just
> fixed, with software keyboard appearing unexpectedly.

I just ran this test with and without having the test force the software keyboard (using `UIHelper.setHardwareKeyboardAttached`), and confirmed that I see the same output in both cases.

[Wenson Hsieh]

(In reply to Wenson Hsieh from comment #12)
> (In reply to Alexey Proskuryakov from comment #11)
> > This makes me wonder if this has the same root cause as the bug Wenson just
> > fixed, with software keyboard appearing unexpectedly.
> 
> I just ran this test with and without having the test force the software
> keyboard (using `UIHelper.setHardwareKeyboardAttached`), and confirmed that
> I see the same output in both cases.

(The same test failure output, that is)

[Wenson Hsieh]

(In reply to Wenson Hsieh from comment #13)
> (In reply to Wenson Hsieh from comment #12)
> > (In reply to Alexey Proskuryakov from comment #11)
> > > This makes me wonder if this has the same root cause as the bug Wenson just
> > > fixed, with software keyboard appearing unexpectedly.
> > 
> > I just ran this test with and without having the test force the software
> > keyboard (using `UIHelper.setHardwareKeyboardAttached`), and confirmed that
> > I see the same output in both cases.
> 
> (The same test failure output, that is)

It seems that this line:

    await UIHelper.selectWordByDoubleTapOrClick(document.getElementById('test'));

…is failing to actually produce a ranged selection.

[Wenson Hsieh]

(In reply to Wenson Hsieh from comment #14)
> (In reply to Wenson Hsieh from comment #13)
> > (In reply to Wenson Hsieh from comment #12)
> > > (In reply to Alexey Proskuryakov from comment #11)
> > > > This makes me wonder if this has the same root cause as the bug Wenson just
> > > > fixed, with software keyboard appearing unexpectedly.
> > > 
> > > I just ran this test with and without having the test force the software
> > > keyboard (using `UIHelper.setHardwareKeyboardAttached`), and confirmed that
> > > I see the same output in both cases.
> > 
> > (The same test failure output, that is)
> 
> It seems that this line:
> 
>     await
> UIHelper.selectWordByDoubleTapOrClick(document.getElementById('test'));
> 
> …is failing to actually produce a ranged selection.

The test seems to start passing if I perturb the double tap location a bit, like so:

diff --git a/LayoutTests/resources/ui-helper.js b/LayoutTests/resources/ui-helper.js
index 3aaba293a2c..49c3d526023 100644
--- a/LayoutTests/resources/ui-helper.js
+++ b/LayoutTests/resources/ui-helper.js
@@ -201,7 +201,8 @@ window.UIHelper = class UIHelper {
         const y = boundingRect.y + relativeY;
         if (this.isIOSFamily()) {
             await UIHelper.activateAndWaitForInputSessionAt(x, y);
-            await UIHelper.doubleTapAt(x, y);
+            await UIHelper.activateAt(x + 1, y + 1);
+            await UIHelper.activateAt(x + 1, y + 1);
             // This is only here to deal with async/sync copy/paste calls, so
             // once <rdar://problem/16207002> is resolved, should be able to remove for faster tests.
             await new Promise(resolve => testRunner.runUIScript("uiController.uiScriptComplete()", resolve));

…this appears to share the same root cause as https://bugs.webkit.org/show_bug.cgi?id=203392. We should really consider fixing position information request caching in WKContentViewInteraction, so that we don’t end up with a stale value for `_positionInformation.nodeAtPositionIsFocusedElement` after focusing an element.

[Wenson Hsieh]

Created attachment 381989 [details]
Patch

[Wenson Hsieh]

Created attachment 381991 [details]
Rename a variable

[Wenson Hsieh]

Comment on attachment 381991 [details]
Rename a variable

> Found 2 new test failures: fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad.html, fast/scrolling/ios/frame-scrolling-no.html (failure)23 analyze-layout-tests-results

I wasn’t able to reproduce these crashes locally.

[Aakash Jain]

(In reply to Wenson Hsieh from comment #18)
> Comment on attachment 381991 [details]
> Rename a variable
> 
> > Found 2 new test failures: fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad.html, fast/scrolling/ios/frame-scrolling-no.html (failure)23 analyze-layout-tests-results
> 
> I wasn’t able to reproduce these crashes locally.
I retried the build, and the tests passed this time.

[Tim Horton]

Comment on attachment 381991 [details]
Rename a variable

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

> Source/WebKit/ChangeLog:31
> +        While this may potentially leads to an additional synchronous position information request when tapping at the
> +        same location after focusing an element, this is very difficult to achieve in practice, since the tap location
> +        would need to be _exactly_ at the same location.

As mentioned on IRC, there is a solution that avoids this pitfall

[Wenson Hsieh]

(In reply to Tim Horton from comment #20)
> Comment on attachment 381991 [details]
> Rename a variable
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=381991&action=review
> 
> > Source/WebKit/ChangeLog:31
> > +        While this may potentially leads to an additional synchronous position information request when tapping at the
> > +        same location after focusing an element, this is very difficult to achieve in practice, since the tap location
> > +        would need to be _exactly_ at the same location.
> 
> As mentioned on IRC, there is a solution that avoids this pitfall

Yep! Investigating it here: https://bugs.webkit.org/show_bug.cgi?id=203498

> IRC

😢

[WebKit Commit Bot]

Comment on attachment 381991 [details]
Rename a variable

Clearing flags on attachment: 381991

Committed r251665: <https://trac.webkit.org/changeset/251665>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

For 

editing/pasteboard/smart-paste-paragraph-002.html
editing/pasteboard/smart-paste-paragraph-003.html
editing/pasteboard/smart-paste-paragraph-004.html


iPhone debug is a flaky failure. Update expectations https://trac.webkit.org/changeset/280305/webkit

[Wenson Hsieh]

(In reply to ayumi_kojima from comment #24)
> For 
> 
> editing/pasteboard/smart-paste-paragraph-002.html
> editing/pasteboard/smart-paste-paragraph-003.html
> editing/pasteboard/smart-paste-paragraph-004.html
> 
> 
> iPhone debug is a flaky failure. Update expectations
> https://trac.webkit.org/changeset/280305/webkit

This Bugzilla bug is pretty old (from 2019), and the radar is closed, indicating that the failures from back then were resolved by this change.

If these tests are starting to fail again, I suspect it might be due to a different root cause.

(In reply to Wenson Hsieh from comment #25)
> (In reply to ayumi_kojima from comment #24)
> > For 
> > 
> > editing/pasteboard/smart-paste-paragraph-002.html
> > editing/pasteboard/smart-paste-paragraph-003.html
> > editing/pasteboard/smart-paste-paragraph-004.html
> > 
> > 
> > iPhone debug is a flaky failure. Update expectations
> > https://trac.webkit.org/changeset/280305/webkit
> 
> This Bugzilla bug is pretty old (from 2019), and the radar is closed,
> indicating that the failures from back then were resolved by this change.
> 
> If these tests are starting to fail again, I suspect it might be due to a
> different root cause.

New bug is filed here: https://bugs.webkit.org/show_bug.cgi?id=228285