673206 - Window resize due to keyboard shouldn't resize content browser windows

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Wesley Johnston (:wesj)]

When the software keyboard pops up in Android, we resize our window to take up the remaining width of the page. This can also result in us changing our cssviewport, causing windows to reflow and the form elements on them to potentially move around. It would be better if we could just avoid resizing (changing the css viewport) in this case.

We won't have to worry about making sure form elements at the bottom of the page are visible, because we will resize browser.xul. We just don't want to call getBrowser().setWindowSize() in this case.

Figuring out how to differentiate the two types of resizes may be the problem here.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

related to bug 612999?

Comment 2

[Wesley Johnston (:wesj)]

Hmm... If I read that bug right, the fix would change the underlying cssViewport (the page would get wider), but the parent would not resize its displayport(? sorry if i"m getting names wrong here). In this bug, the cssviewport would not change when the keyboard appears, but we would change the width and height or browser.xul (not sure we have a fancy name for that?)

I was primarily noticing times when the form assistant is pointing to the wrong place. This can happen when the page layout changes due to the keyboard appearing. A message is sent to the parent telling it that the element moved, which can trigger a rezoom to recenter the element, which can trigger the ContentPopupHelper to move the popup. However, the ContentPopupHelper needs to be careful that it is using the new element rect, which right now, doesn't always seem to be true (but sometimes it does!).

I put this up as an "alternative" because it should fix that, and also reduce some of the jerkyness we see.

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Patch

We already had some code lying around to detect if this was due to keyboard or not.

Comment 4

[Wesley Johnston (:wesj)]

Comment on attachment 547528 [details] [diff] [review]
Patch

Oh wait... that keyboard check is a bit... different than I though. Need to think about this for a second. Removing review request.

Comment 5

[Wesley Johnston (:wesj)]

Attachment: Patch v2

Updated this check to be a bit more generic (wish we had something better).

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #5)
> Created attachment 548495 [details] [diff] [review] [review]
> Patch v2
> 
> Updated this check to be a bit more generic (wish we had something better).

Looks like you don't want to do |if (isDueToKeyboard)| otherwise you will end up doing thing only when it's the keyboard changes which is the opposite of the bug title? Or do i miss something?

Comment 7

[Wesley Johnston (:wesj)]

Whoops. Yep. A lazy last minute change. Fixing (and testing)...

Comment 8

[Wesley Johnston (:wesj)]

Attachment: Patch v2.1

Updated.

Comment 9

[Wesley Johnston (:wesj)]

Attachment: Path v2.1.1....1

And the qrefed...

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 548529 [details] [diff] [review]
Path v2.1.1....1

(In reply to comment #0) 
> We won't have to worry about making sure form elements at the bottom of the
> page are visible, because we will resize browser.xul. We just don't want to
> call getBrowser().setWindowSize() in this case.

This will potentially broke on Meego where the keyboard does not resize the windows but we can see if somebody complains about it one day... (which I doubt)

Comment 11

[Wesley Johnston (:wesj)]

inbound
http://hg.mozilla.org/integration/mozilla-inbound/rev/35bd1513465d

Comment 12

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/35bd1513465d