Bug 200902 - [iOS] Should show input view when became first responder if keyboard was showing when the view was resigned
Summary: [iOS] Should show input view when became first responder if keyboard was show...
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks: 190571
  Show dependency treegraph
 
Reported: 2019-08-19 16:32 PDT by Daniel Bates
Modified: 2020-03-25 10:12 PDT (History)
3 users (show)

See Also:


Attachments
Work-in-progress (21.13 KB, patch)
2019-08-19 16:34 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch and layout tests (30.47 KB, patch)
2019-08-20 11:04 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
To land (31.46 KB, patch)
2019-08-21 15:36 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
To land (31.41 KB, patch)
2019-08-22 15:21 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
To Land (32.62 KB, patch)
2020-03-25 10:12 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2019-08-19 16:32:33 PDT
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.
Comment 1 Daniel Bates 2019-08-19 16:32:58 PDT
<rdar://problem/54231756>
Comment 2 Daniel Bates 2019-08-19 16:34:49 PDT
Created attachment 376716 [details]
Work-in-progress
Comment 3 Daniel Bates 2019-08-20 11:04:14 PDT
Created attachment 376782 [details]
Patch and layout tests
Comment 4 Wenson Hsieh 2019-08-20 15:43:31 PDT
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.
Comment 5 Daniel Bates 2019-08-20 16:44:14 PDT
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.
Comment 6 Daniel Bates 2019-08-21 15:36:27 PDT
Created attachment 376933 [details]
To land
Comment 7 Daniel Bates 2019-08-22 15:21:12 PDT
Created attachment 377059 [details]
To land

Rebased patch
Comment 8 Daniel Bates 2019-08-22 15:23:00 PDT
Committed r249031: <https://trac.webkit.org/changeset/249031>
Comment 9 Russell Epstein 2019-08-23 10:15:57 PDT
Reverted r249031 for reason:

Causes multiple test failures on iOS simulator

Committed r249051: <https://trac.webkit.org/changeset/249051>
Comment 10 Daniel Bates 2020-03-25 10:10:34 PDT
(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.
Comment 11 Daniel Bates 2020-03-25 10:12:31 PDT
Created attachment 394514 [details]
To Land