Bug 163581 – REGRESSION (r201471): Keyboard remains visible when swiping back on twitter.com

• Component: UI Events
• Status: RESOLVED FIXED

[Ryosuke Niwa]

Reproduction steps:
1. Login to twitter.com
2. Click "reply" on any tweet
3. Tap on the text field to bring up keyboard
4. Swipe back

Expected result: Keyboard goes away
Actual result: Keyboard stays open

[Ryosuke Niwa]

<rdar://problem/27739558>

8/16/16, 1:54 PM Andy Estes:
Hiding the keyboard is a side effect of dispatching the blur event. In Element::dispatchBlurEvent(), we call ChromeClient::elementDidBlur(), which sends the StopAssistingNode message to the UI process, which eventually calls -[WKContentView _stopAssistingNode].

[Ryosuke Niwa]

Created attachment 291912 [details]
Fixes the bug

[Simon Fraser (smfr)]

Comment on attachment 291912 [details]
Fixes the bug

This is testable, via UIScriptController. Please ad a test.

[Ryosuke Niwa]

(In reply to comment #3)
> Comment on attachment 291912 [details]
> Fixes the bug
> 
> This is testable, via UIScriptController. Please ad a test.

Does it bring up a real keyboard?

[Simon Fraser (smfr)]

Yes, and you get callbacks when the keyboard appears and disappears. See tests in fast/forms/ios

[Ryosuke Niwa]

(In reply to comment #5)
> Yes, and you get callbacks when the keyboard appears and disappears. See
> tests in fast/forms/ios

Oh, neat!

[Ryosuke Niwa]

Huh, apparently all the tests in fast/forms/ios are skipped at the moment because we don't override [ Skip ] expectation in LayoutTests/platform/ios-simulator-wk2/TestExpectations :(

[Ryosuke Niwa]

Created attachment 291922 [details]
Added a test

[Ryosuke Niwa]

Created attachment 291926 [details]
Reverted unrelated change

[Simon Fraser (smfr)]

Comment on attachment 291926 [details]
Reverted unrelated change

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

> Source/WebCore/dom/Document.cpp:3676
> +            // Match the order in HTMLTextFormControlElement::dispatchBlurEvent.
>              if (is<HTMLInputElement>(*oldFocusedElement))
>                  downcast<HTMLInputElement>(*oldFocusedElement).endEditing();
> +            if (page())
> +                page()->chrome().client().elementDidBlur(oldFocusedElement.get());

This is pretty confusing. Element::dispatchBlurEvent() calls chrome().client().elementDidBlur() BEFORE dispatching the blur event.

If I look at HTMLTextFormControlElement::dispatchBlurEvent(), it doesn't have any calls to chrome().client().elementDidBlur(), but calls through to TextFieldInputType::handleBlurEvent() (note the *handle*) which calls endEditing(). So I can't tell from the comment whether the ordering is correct. I wish there were a single choke point for this blur handling.

[Ryosuke Niwa]

Comment on attachment 291926 [details]
Reverted unrelated change

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

>> Source/WebCore/dom/Document.cpp:3676
>> +                page()->chrome().client().elementDidBlur(oldFocusedElement.get());
> 
> This is pretty confusing. Element::dispatchBlurEvent() calls chrome().client().elementDidBlur() BEFORE dispatching the blur event.
> 
> If I look at HTMLTextFormControlElement::dispatchBlurEvent(), it doesn't have any calls to chrome().client().elementDidBlur(), but calls through to TextFieldInputType::handleBlurEvent() (note the *handle*) which calls endEditing(). So I can't tell from the comment whether the ordering is correct. I wish there were a single choke point for this blur handling.

We don't want to dispatch blur event here. That's the whole point of the original patch.
I intend to refactor this code once this patch is landed but this needs to be merged into a branch
so I'm not interested in doing that kind of a large code change here,

[WebKit Commit Bot]

Comment on attachment 291926 [details]
Reverted unrelated change

Clearing flags on attachment: 291926

Committed r207486: <http://trac.webkit.org/changeset/207486>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.