Bug 200902 – [iOS] Should show input view when became first responder if keyboard was showing when the view was resigned

• Component: WebKit Misc.
• Status: REOPENED

[Daniel Bates]

We should remember whether the keyboard is showing when the view resigns first responder so that we can show the keyboard (again) when the view becomes first responder. On a iPad without a hardware keyboard, this makes the experience more pleasant when switching back and forth between a tab with a focused editable elements, say like a Google Docs document, and another tab.

[Daniel Bates]

<rdar://problem/54231756>

[Daniel Bates]

Created attachment 376716 [details]
Work-in-progress

[Daniel Bates]

Created attachment 376782 [details]
Patch and layout tests

[Wenson Hsieh]

Comment on attachment 376782 [details]
Patch and layout tests

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

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:341
> +    BOOL _shouldShowInputViewForPageReActivation;

Nit - this should probably be …Reactivation instead of …ReActivation.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:5277
> +    SetForScope<BOOL> shouldShowInputViewForPageReActivationScope { _shouldShowInputViewForPageReActivation, startInputSessionPolicy == _WKFocusStartsInputSessionPolicyAuto && (self.isFirstResponder || _becomingFirstResponder) && activityStateChanges.contains(WebCore::ActivityState::IsFocused) && _wasResignedWhileShowingInputView };

Nit - we could consider factoring out the self.isFirstResponder || _becomingFirstResponder check into a separate method, and just use the helper in both places.

[Daniel Bates]

Thanks for the review.

(In reply to Wenson Hsieh from comment #4)
> Comment on attachment 376782 [details]
> Patch and layout tests
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=376782&action=review
> 
> > Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:341
> > +    BOOL _shouldShowInputViewForPageReActivation;
> 
> Nit - this should probably be …Reactivation instead of …ReActivation.
> 

Yep, will fix.

> > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:5277
> > +    SetForScope<BOOL> shouldShowInputViewForPageReActivationScope { _shouldShowInputViewForPageReActivation, startInputSessionPolicy == _WKFocusStartsInputSessionPolicyAuto && (self.isFirstResponder || _becomingFirstResponder) && activityStateChanges.contains(WebCore::ActivityState::IsFocused) && _wasResignedWhileShowingInputView };
> 
> Nit - we could consider factoring out the self.isFirstResponder ||
> _becomingFirstResponder check into a separate method, and just use the
> helper in both places.

Will factor out.

[Daniel Bates]

Created attachment 376933 [details]
To land

[Daniel Bates]

Created attachment 377059 [details]
To land

Rebased patch

[Daniel Bates]

Committed r249031: <https://trac.webkit.org/changeset/249031>

[Russell Epstein]

Reverted r249031 for reason:

Causes multiple test failures on iOS simulator

Committed r249051: <https://trac.webkit.org/changeset/249051>

[Daniel Bates]

(In reply to Russell Epstein from comment #9)
> Reverted r249031 for reason:
> 
> Causes multiple test failures on iOS simulator
> 
> Committed r249051: <https://trac.webkit.org/changeset/249051>

To elaborate more, the reason why the change was rolled out was because it revealed a UIKit bug, <rdar://problem/55201802>. That bug has been fixed in 13.4.

[Daniel Bates]

Created attachment 394514 [details]
To Land