Bug 197532

Summary: REGRESSION: Layout test editing/selection/ios/selection-after-changing-text-with-callout-menu.html is failing
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: HTML EditingAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, megan_gardner, rniwa, sroberts, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
rniwa: review+
Fix GTK/WPE/Win none

Description Wenson Hsieh 2019-05-02 15:03:26 PDT
<rdar://problem/50177144>
Comment 1 Wenson Hsieh 2019-05-02 16:30:53 PDT
Created attachment 368841 [details]
Patch
Comment 2 Ryosuke Niwa 2019-05-02 16:38:49 PDT
Comment on attachment 368841 [details]
Patch

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

> Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm:1110
> -void WebPageProxy::editorStateChanged(const EditorState& editorState)
> +void WebPageProxy::setEditorState(const EditorState& editorState)

Maybe updateEditorState? It's not like we're setting a new editor state object.
EditorState is always there but we're updating its content.
Comment 3 Wenson Hsieh 2019-05-02 17:24:50 PDT
Created attachment 368850 [details]
Fix GTK/WPE/Win
Comment 4 WebKit Commit Bot 2019-05-02 18:05:37 PDT
Comment on attachment 368850 [details]
Fix GTK/WPE/Win

Clearing flags on attachment: 368850

Committed r244897: <https://trac.webkit.org/changeset/244897>
Comment 5 Shawn Roberts 2019-05-03 09:17:48 PDT
It appears appears after the changes in https://trac.webkit.org/changeset/244897

We have 4 API time outs on iOS Simulator and iOS Simulator Debug testers. It appears that these same failures were observed by the new API-iOS EWS

They are flaky Timeouts, but I've been able to verify them all locally with r244897, and they all pass with r244895

TestWebKitAPI.KeyboardInputTests.OverrideInputAssistantItemBarButtonGroups TestWebKitAPI.KeyboardInputTests.CaretSelectionRectAfterRestoringFirstResponderWithRetainActiveFocusedState TestWebKitAPI.KeyboardInputTests.ModifyInputAssistantItemBarButtonGroups TestWebKitAPI.KeyboardInputTests.CaretSelectionRectAfterRestoringFirstResponder

Verified with :

run-api-tests TestWebKitAPI.KeyboardInputTests.OverrideInputAssistantItemBarButtonGroups TestWebKitAPI.KeyboardInputTests.CaretSelectionRectAfterRestoringFirstResponderWithRetainActiveFocusedState TestWebKitAPI.KeyboardInputTests.ModifyInputAssistantItemBarButtonGroups TestWebKitAPI.KeyboardInputTests.CaretSelectionRectAfterRestoringFirstResponder --debug --ios-simulator

2 of the tests timeout with the following error:

TestWebKitAPI.KeyboardInputTests.CaretSelectionRectAfterRestoringFirstResponderWithRetainActiveFocusedState
        2019-05-03 09:15:48.831 TestWebKitAPI[6692:429492] Expected a caret rect of {{16, 13}, {2, 15}}, but still observed {{16, 13}, {3, 15}}

    TestWebKitAPI.KeyboardInputTests.CaretSelectionRectAfterRestoringFirstResponder
        2019-05-03 09:15:19.193 TestWebKitAPI[6650:425896] Expected a caret rect of {{16, 13}, {2, 15}}, but still observed {{16, 13}, {3, 15}}
Comment 6 Wenson Hsieh 2019-05-03 09:18:36 PDT
(In reply to Shawn Roberts from comment #5)
> It appears appears after the changes in
> https://trac.webkit.org/changeset/244897
> 
> We have 4 API time outs on iOS Simulator and iOS Simulator Debug testers. It
> appears that these same failures were observed by the new API-iOS EWS
> 
> They are flaky Timeouts, but I've been able to verify them all locally with
> r244897, and they all pass with r244895
> 
> TestWebKitAPI.KeyboardInputTests.OverrideInputAssistantItemBarButtonGroups
> TestWebKitAPI.KeyboardInputTests.
> CaretSelectionRectAfterRestoringFirstResponderWithRetainActiveFocusedState
> TestWebKitAPI.KeyboardInputTests.ModifyInputAssistantItemBarButtonGroups
> TestWebKitAPI.KeyboardInputTests.
> CaretSelectionRectAfterRestoringFirstResponder
> 
> Verified with :
> 
> run-api-tests
> TestWebKitAPI.KeyboardInputTests.OverrideInputAssistantItemBarButtonGroups
> TestWebKitAPI.KeyboardInputTests.
> CaretSelectionRectAfterRestoringFirstResponderWithRetainActiveFocusedState
> TestWebKitAPI.KeyboardInputTests.ModifyInputAssistantItemBarButtonGroups
> TestWebKitAPI.KeyboardInputTests.
> CaretSelectionRectAfterRestoringFirstResponder --debug --ios-simulator
> 
> 2 of the tests timeout with the following error:
> 
> TestWebKitAPI.KeyboardInputTests.
> CaretSelectionRectAfterRestoringFirstResponderWithRetainActiveFocusedState
>         2019-05-03 09:15:48.831 TestWebKitAPI[6692:429492] Expected a caret
> rect of {{16, 13}, {2, 15}}, but still observed {{16, 13}, {3, 15}}
> 
>    
> TestWebKitAPI.KeyboardInputTests.
> CaretSelectionRectAfterRestoringFirstResponder
>         2019-05-03 09:15:19.193 TestWebKitAPI[6650:425896] Expected a caret
> rect of {{16, 13}, {2, 15}}, but still observed {{16, 13}, {3, 15}}

I'm looking into this now.
Comment 7 Wenson Hsieh 2019-05-03 10:17:17 PDT
> 2 of the tests timeout with the following error:
> 
> TestWebKitAPI.KeyboardInputTests.
> CaretSelectionRectAfterRestoringFirstResponderWithRetainActiveFocusedState
>         2019-05-03 09:15:48.831 TestWebKitAPI[6692:429492] Expected a caret
> rect of {{16, 13}, {2, 15}}, but still observed {{16, 13}, {3, 15}}
> 
>    
> TestWebKitAPI.KeyboardInputTests.
> CaretSelectionRectAfterRestoringFirstResponder
>         2019-05-03 09:15:19.193 TestWebKitAPI[6650:425896] Expected a caret
> rect of {{16, 13}, {2, 15}}, but still observed {{16, 13}, {3, 15}}

So for these two tests, I think it's actually a progression?

Bizarrely, I changed the expectation from {{ 16, 13 }, { 3, 15 }} to {{ 16, 13 }, { 2, 15 }} in r234614 because I thought the width of 3 was before transforming to content view coordinates. However, logging shows that the EditorState's caret rect is indeed {{ 16, 13 }, { 3, 15 }} in content coordinates, so {{ 16, 13 }, { 3, 15 }} actually seems right.

Need to understand why this changed...
Comment 8 Wenson Hsieh 2019-05-03 14:31:40 PDT
(In reply to Wenson Hsieh from comment #7)
> > 2 of the tests timeout with the following error:
> > 
> > TestWebKitAPI.KeyboardInputTests.
> > CaretSelectionRectAfterRestoringFirstResponderWithRetainActiveFocusedState
> >         2019-05-03 09:15:48.831 TestWebKitAPI[6692:429492] Expected a caret
> > rect of {{16, 13}, {2, 15}}, but still observed {{16, 13}, {3, 15}}
> > 
> >    
> > TestWebKitAPI.KeyboardInputTests.
> > CaretSelectionRectAfterRestoringFirstResponder
> >         2019-05-03 09:15:19.193 TestWebKitAPI[6650:425896] Expected a caret
> > rect of {{16, 13}, {2, 15}}, but still observed {{16, 13}, {3, 15}}
> 
> So for these two tests, I think it's actually a progression?
> 
> Bizarrely, I changed the expectation from {{ 16, 13 }, { 3, 15 }} to {{ 16,
> 13 }, { 2, 15 }} in r234614 because I thought the width of 3 was before
> transforming to content view coordinates. However, logging shows that the
> EditorState's caret rect is indeed {{ 16, 13 }, { 3, 15 }} in content
> coordinates, so {{ 16, 13 }, { 3, 15 }} actually seems right.
> 
> Need to understand why this changed...

I think these two tests caught a legitimate regression; going to track this in https://bugs.webkit.org/show_bug.cgi?id=197579.