1183205 - Incorrect keyboard height calculation

Status: RESOLVED FIXED

(Firefox for iOS :: General, defect)

Description

[Aaron Train [:aaronmt]]

Attachment: Screen Shot 2015-07-13 at 1.52.10 PM.png

See screenshot

Comment 1

[Vlad Filippov (:vladikoff)]

Related issue filed in https://github.com/mozilla/fxa-content-server/issues/2751

Comment 2

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

This has bugged me for a while. This is due to bad keyboard handling on our part.

Comment 3

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

This also happens when you have an external keyboard attached. Or when you run in the simulator with the on-screen keyboard disabled.

Comment 4

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Attachment: PR: https://github.com/mozilla/firefox-ios/pull/739

This patch changes the `KeyboardHelper` to have a `intersectionHeightForView()` method instead of a `height` property.

> Return the height of the keyboard that overlaps with the specified view. This is more accurate than simply using the height of UIKeyboardFrameBeginUserInfoKey since for example on iPad the overlap may be partial or if an external keyboard is attached, the intersection height will be zero. (Even if the height of the *invisible* keyboard will look normal!)

This fixes the case where the FxA login has a abnormal bottom inset on iPad and it also fixes the case where all our views that deal with the keyboard (Search, Snackbar, FxA) have a large bottom inset when a virtual or external keyboard is used.

Comment 5

[Richard Newman [:rnewman]]

Comment on attachment 8633850 [details] [review]
PR: https://github.com/mozilla/firefox-ios/pull/739

Drive-by. sleroux can find what I missed if he chooses to :D

Comment 6

[Stephan Leroux [:sleroux]]

Comment on attachment 8633850 [details] [review]
PR: https://github.com/mozilla/firefox-ios/pull/739

I'm still seeing an issue in the iPad Air 8.4 simulator when using the software keyboard where there's a gap between the bottom of the signin white background and the keyboard. Also, when you dismiss the keyboard, it seems to zoom into the webview that's displaying the signin page.

STR:
(To see soft keyboard, turn off "Connect hardware keyboard" under the simulator's Hardware menu)

1. Launch iPad Air 8.4 simulator in landscape
2. Navigate to Sync tab
3. Tap signin button
4. Tap signin
5. Tap email input field

Also, whenever I see a mysterious 20px gap I usually associate it with a calculation related to the status bar (yay fun). Try toggling the in-call status bar state in the simulator and see if this changes to 40px. If it is a status bar issue, it might be related to similar issue I'm looking at for the tab tray controller.

Comment 7

[Stephan Leroux [:sleroux]]

Attachment: iOS Simulator Screen Shot Jul 15, 2015, 9.27.28 AM.png

Comment 8

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Ugh I landed this before I saw your comment. I will leave this bug open and post a followup fix.

Weird. I really tested this well on the iPad Simulator. But maybe not on the iPad Air simulator. I wonder if retina vs non-retina makes a difference.

Comment 9

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Test plan for iPhone:

1) Start iPhone 6 simulator with on-screen keyboard enabled

1.1) Search:
* Tap in location bar. Menu should popup.
* Type in location bar. Search results should appear. Quick search bar should be attached to top of keyboard.
* Hit cancel. Menu should go away. Quick search bar should go away.

1.2) Snackbars
* Go to news.ycombinator.com
* Hit the login link
* Select the username field - Menu should appear
* Login with foo/foo (non existent). Hit login. Menu should go away. Snackbar should appear, attached to bottom of screen.
* Tap in a text field - Menu should appear. Snackbar should move up together with the menu. (THere is a 20 pixel gap which is a bug to be filed)
* Hit Done. Menu should go away, snackbar should move down.
* Hit Not Now, snackbar goes away

1.3) FxA Login
* Go to settings, hit "Sign In" - Firefox Sync login web page appears
* Tap in the email field. Menu apears.
* Scroll the web view. Notice that the view has resized. No gap at bottom.

2) Start iPhone 6 simulator with on-screen keyboard disabled

2.1) Search:
* Tap in location bar. No maenu should popup.
* Type in location bar. Search results should appear. Quick search bar should be attached to bottom of screen. No keyboard visible.
* Hit cancel. Quick search search bar should go away.

2.2) Snackbars
* Go to news.ycombinator.com
* Hit the login link
* Select the username field - Only the bar with [< > Done] should appear at the bottom of the screen
* Login with foo/foo (non existent). Hit login. Bar [< > Done] should go away. Snackbar should appear, attached to bottom of screen.
* Tap in a text field - The bar [< > Done] should appear. Snackbar should move up together with the bar. (THere is a 20 pixel gap which is a bug to be filed)
* Hit Done. Menu should go away, snackbar should move down.
* Hit Not Now, snackbar goes away

2.3) FxA Login
* Go to settings, hit "Sign In" - Firefox Sync login web page appears
* Tap in the email field. On the [<> Done] bar appears.
* Depending on phone size. Scroll the web view. (May fit on 6 and up) Notice that the view has resized. No gap at bottom.

Comment 10

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Attachment: PR: https://github.com/mozilla/firefox-ios/pull/748

This sits on top of PR 739 which I landed too soon.

Second PR for this bug. Introduced a KeyboardHelper delegate method that handles NSKeyboardDidShow. This only seems to be needed on iPad.

I tried to make the delegate methods optional but that fails because the protocol and keyboard state are not ObjC compatible. (Only ObjC compatible protocols can be optional). I chose the easy way here and simply added the new (empty) delegate method to the other two places where we use the KeyboardHelper.

Comment 11

[Stephan Leroux [:sleroux]]

Comment on attachment 8634347 [details] [review]
PR: https://github.com/mozilla/firefox-ios/pull/748

Just left a comment about allowing optional protocol if you want it.

Comment 12

[Richard Newman [:rnewman]]

Land this please, Steph!

Comment 13

[Stephan Leroux [:sleroux]]

Merged