Bug 193786 – Turn on Smart Paste

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Megan Gardner]

Turn on Smart Paste from iOS Pasteboard

[Megan Gardner]

Created attachment 360133 [details]
Patch

[EWS Watchlist]

Comment on attachment 360133 [details]
Patch

Attachment 360133 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10889432

New failing tests:
editing/pasteboard/smart-paste-007.html

[EWS Watchlist]

Created attachment 360143 [details]
Archive of layout-test-results from ews103 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 360133 [details]
Patch

Attachment 360133 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10889760

New failing tests:
editing/pasteboard/smart-paste-007.html

[EWS Watchlist]

Created attachment 360145 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Ryosuke Niwa]

Comment on attachment 360133 [details]
Patch

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

> LayoutTests/editing/pasteboard/smart-paste-001.html:49
> +    if (UIHelper.isIOS())
> +        getSelection().setBaseAndExtent(test.firstChild, 1, test.firstChild, 5);
> +    else
> +        doubleClickAtSelectionStart();

We shouldn't have this kind of platform-branches in tests.
We should abstract UIHelper.doubleTapAt so that there is a single UIHelper function like
UIHelper.selectWordBoundaryByActivationAt(x, y)

An alternative approach is to use getSelection().modify('extend', 'forward', 'wordboundary')

r- because this makes all these tests behave differently in iOS compared to other platforms.

[Ryosuke Niwa]

To put money where my mouth is, I have a test code change which makes it work across platforms. Give me another half an hour to clean things up & post the change.

[Ryosuke Niwa]

Created attachment 360187 [details]
Layout test changes needed

[Ryosuke Niwa]

Created attachment 360197 [details]
With Mac rebaselines

Because of updates to the test, we'd have to rebaselien some results in macOS as well.

Perhaps we should update the tests in a separate patch.

[Megan Gardner]

Created attachment 360201 [details]
Patch

[Ryosuke Niwa]

Askin Wenson for a review since I wrote the test change.

[Ryosuke Niwa]

Oh, the latest patch is missing my macOS rebaselines. Just merge changes in https://bug-193786-attachments.webkit.org/attachment.cgi?id=360187 and the patch should be good to go.

[EWS Watchlist]

Comment on attachment 360201 [details]
Patch

Attachment 360201 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10896520

New failing tests:
editing/pasteboard/smart-paste-002.html
editing/pasteboard/smart-paste-005.html
editing/pasteboard/smart-paste-006.html
editing/pasteboard/smart-paste-008.html

[EWS Watchlist]

Created attachment 360211 [details]
Archive of layout-test-results from ews102 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Wenson Hsieh]

Comment on attachment 360201 [details]
Patch

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

r=me, provided EWS is happy. It looks like either macOS needs rebaselined expectations, or the tests need to be tweaked in such a way that they fulfill existing expectations.

> Source/WebCore/ChangeLog:8
> +        Turned on a modified tests:

Perhaps something like, "Enabled and modified existing tests:"?

> LayoutTests/editing/pasteboard/smart-paste-008.html:16
> +    await UIHelper.selectWordByDoubleTapOrClick(document.getElementById('test'));

Nit - UIHelper.selectWordByDoubleTapOrClick(e);

> LayoutTests/platform/ios/TestExpectations:2140
> +

Nit - stray newline.

[EWS Watchlist]

Comment on attachment 360201 [details]
Patch

Attachment 360201 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10896638

New failing tests:
editing/pasteboard/smart-paste-002.html
editing/pasteboard/smart-paste-005.html
editing/pasteboard/smart-paste-006.html
editing/pasteboard/smart-paste-008.html

[EWS Watchlist]

Created attachment 360214 [details]
Archive of layout-test-results from ews112 for mac-highsierra

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

[EWS Watchlist]

Comment on attachment 360201 [details]
Patch

Attachment 360201 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10896639

New failing tests:
editing/pasteboard/paste-line-endings-008.html
editing/pasteboard/input-field-1.html
editing/pasteboard/paste-text-019.html
editing/pasteboard/paste-line-endings-001.html
editing/pasteboard/paste-xml.xhtml
editing/pasteboard/paste-line-endings-007.html
editing/pasteboard/paste-text-002.html
editing/pasteboard/paste-text-010.html
editing/pasteboard/paste-text-009.html
editing/pasteboard/paste-line-endings-006.html
editing/pasteboard/smart-paste-007.html
editing/pasteboard/paste-match-style-001.html
editing/pasteboard/paste-line-endings-010.html
editing/pasteboard/onpaste-text-html.html
editing/pasteboard/paste-text-at-tabspan-003.html
editing/pasteboard/paste-line-endings-009.html
editing/pasteboard/paste-text-003.html
editing/pasteboard/paste-text-008.html
editing/pasteboard/copy-paste-first-line-in-textarea.html
editing/pasteboard/4989774.html
editing/pasteboard/smart-paste-008.html
editing/pasteboard/paste-text-001.html
editing/pasteboard/4076267-2.html
editing/pasteboard/paste-text-at-tabspan-001.html
editing/pasteboard/smart-paste-003-trailing-whitespace.html

[EWS Watchlist]

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

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

[EWS Watchlist]

Comment on attachment 360201 [details]
Patch

Attachment 360201 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10897247

New failing tests:
editing/pasteboard/smart-paste-002.html
editing/pasteboard/smart-paste-005.html
editing/pasteboard/smart-paste-006.html
http/tests/security/anchor-download-block-crossorigin.html
editing/pasteboard/smart-paste-008.html

[EWS Watchlist]

Created attachment 360218 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Wenson Hsieh]

Comment on attachment 360201 [details]
Patch

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

ue

> LayoutTests/resources/ui-helper.js:124
> +            await UIHelper.ensurePresentationUpdate();

I found this line very confusing, because I couldn't think of any reason why it should be necessarily to wait for a layer tree commit before continuing the test. But in both my (and Ryosuke's) testing, removing this causes these tests to be flaky! So I looked into this some more. In the case where the test fails after removing the call to UIHelper.ensurePresentationUpdate(), I observed the following sequence of events:

1. HID events for double tap are synthesized and dispatched to the UIApp.
2. The text interaction assistant's double tap gesture recognizer is fired, and it tells WKContentView to -changeSelectionWithGestureAt:…:, which then sends an IPC message to the web process.
3. The web process receives this message, selects the word at the location, and sends an IPC responds back to the UI process.

So far, this is pretty normal. Now here's where things get interesting...

4. When the UI process receives the IPC message (WebPageProxy::gestureCallback), it calls into UIKit, which then calls back into us, asking for an autocorrection context (-[WKContentView requestAutocorrectionContextWithCompletionHandler:]).
5. -[WKContentView requestAutocorrectionContextWithCompletionHandler:] actually blocks on sync IPC back into the web process by calling WebPageProxy::getAutocorrectionContext. While this is happening, UIKit blocks the main thread of the UI process.
6. While the UI process is blocked, let's suppose the promise returned by UIHelper.selectWordByDoubleTapOrClick is now resolved. We'll then attempt to execute "copy" and then "paste" while the UI process is still blocked on sync IPC (!)
7. Since the UI process is waiting on sync IPC, we'll resolve any sync IPC messages we get from the web process before we try and respond to non-blocking IPC messages in order to avoid a permanent deadlock.
8. Since "paste" sends sync IPC while "copy" just sends a regular IPC message, this means the UI process will handle the IPC to paste *before* it gets the IPC message to copy! This, of course, causes the test to fail because we'll read from the pasteboard before we've written to it.

A few more remarks:
- Without the ensurePresentationUpdate(), this test is flaky; in the case where it passes, the promise in step (6) is resolved after the sync IPC call for the autocorrection context.
- The FIXME in -requestAutocorrectionContextWithCompletionHandler: (<rdar://problem/16207002>) tracks making the autocorrection context request async.
- This also means the ensurePresentationUpdate() isn't actually necessary; we just need to make sure that the sync message for the autocorrection context is finished by the time we try and continue the test. This should be doable by ensuring a round trip to the UI process and back (rather than requiring layout and a compositing flush). Something like this: `await new Promise(resolve => testRunner.runUIScript("uiController.uiScriptComplete()", resolve))`.

[Ryosuke Niwa]

Nice analysis!

[Megan Gardner]

Created attachment 360369 [details]
Patch

[Megan Gardner]

Created attachment 360371 [details]
Patch

[EWS Watchlist]

Comment on attachment 360371 [details]
Patch

Attachment 360371 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10927782

New failing tests:
http/tests/security/anchor-download-block-crossorigin.html

[EWS Watchlist]

Created attachment 360377 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Megan Gardner]

Created attachment 360379 [details]
Patch

[EWS Watchlist]

Comment on attachment 360379 [details]
Patch

Attachment 360379 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10935469

New failing tests:
editing/pasteboard/4944770-2.html
editing/pasteboard/paste-line-endings-008.html
editing/pasteboard/input-field-1.html
editing/pasteboard/paste-text-019.html
editing/pasteboard/paste-line-endings-001.html
editing/pasteboard/paste-xml.xhtml
editing/pasteboard/paste-line-endings-007.html
editing/pasteboard/paste-text-002.html
editing/pasteboard/paste-text-010.html
editing/pasteboard/paste-text-009.html
editing/pasteboard/paste-line-endings-006.html
editing/pasteboard/smart-paste-007.html
editing/pasteboard/paste-match-style-001.html
editing/pasteboard/paste-line-endings-010.html
editing/pasteboard/onpaste-text-html.html
editing/pasteboard/paste-text-at-tabspan-003.html
editing/pasteboard/paste-line-endings-009.html
editing/pasteboard/paste-text-003.html
editing/pasteboard/paste-text-008.html
editing/pasteboard/copy-paste-first-line-in-textarea.html
editing/pasteboard/4989774.html
editing/pasteboard/smart-paste-008.html
editing/pasteboard/paste-text-001.html
editing/pasteboard/4076267-2.html
editing/pasteboard/paste-text-at-tabspan-001.html
editing/pasteboard/smart-paste-003-trailing-whitespace.html

[EWS Watchlist]

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

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

[Megan Gardner]

Created attachment 360817 [details]
Patch

[EWS Watchlist]

Comment on attachment 360817 [details]
Patch

Attachment 360817 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10985056

New failing tests:
editing/pasteboard/4944770-2.html
editing/pasteboard/smart-paste-008.html

[EWS Watchlist]

Created attachment 360822 [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.13.6

[Megan Gardner]

Created attachment 360916 [details]
Patch

[Megan Gardner]

Created attachment 360937 [details]
Patch

[EWS Watchlist]

Comment on attachment 360916 [details]
Patch

Attachment 360916 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11001270

New failing tests:
editing/pasteboard/smart-paste-008.html

[EWS Watchlist]

Created attachment 360943 [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.13.6

[EWS Watchlist]

Comment on attachment 360937 [details]
Patch

Attachment 360937 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11003178

New failing tests:
editing/pasteboard/smart-paste-008.html

[EWS Watchlist]

Created attachment 360958 [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.13.6

[Megan Gardner]

Created attachment 360999 [details]
Patch

[Megan Gardner]

Created attachment 361000 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 361000 [details]
Patch for landing

Clearing flags on attachment: 361000

Committed r240902: <https://trac.webkit.org/changeset/240902>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/47768675>

[Truitt Savell]

It appears that the changes in https://trac.webkit.org/changeset/240902/webkit

Has caused one API failure on iOS. 


Failed

    TestWebKitAPI.WKAttachmentTests.InsertAndRemoveDuplicateAttachment
        2019-02-03 02:19:51.653 TestWebKitAPI[81810:95344208] Expected removed attachments: (
        ) to match (
            "<_WKAttachment 0x7fa71030c230 id='c098c0a4-33a3-431a-b4da-317bda9d4003'>"
        ).
        
        /Volumes/Data/slave/ios-simulator-12-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:145
        Value of: removedAttachmentsMatch
          Actual: false
        Expected: true

[Truitt Savell]

iOS run failure: https://build.webkit.org/builders/Apple%20iOS%2012%20Simulator%20Release%20WK2%20%28Tests%29/builds/2385

[Wenson Hsieh]

(In reply to Truitt Savell from comment #45)
> It appears that the changes in
> https://trac.webkit.org/changeset/240902/webkit
> 
> Has caused one API failure on iOS. 
> 
> 
> Failed
> 
>     TestWebKitAPI.WKAttachmentTests.InsertAndRemoveDuplicateAttachment
>         2019-02-03 02:19:51.653 TestWebKitAPI[81810:95344208] Expected
> removed attachments: (
>         ) to match (
>             "<_WKAttachment 0x7fa71030c230
> id='c098c0a4-33a3-431a-b4da-317bda9d4003'>"
>         ).
>         
>        
> /Volumes/Data/slave/ios-simulator-12-release/build/Tools/TestWebKitAPI/Tests/
> WebKitCocoa/WKAttachmentTests.mm:145
>         Value of: removedAttachmentsMatch
>           Actual: false
>         Expected: true

I have a fix for this in https://bugs.webkit.org/show_bug.cgi?id=194207

[Truitt Savell]

It looks like:
editing/pasteboard/smart-paste-007.html
editing/pasteboard/smart-paste-008.html

are flakey failures on iOS Debug WK2. It is still failing constantly on Release WK2

History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=editing%2Fpasteboard%2Fsmart-paste-008.html%20editing%2Fpasteboard%2Fsmart-paste-007.html