855362 - Focused form inputs can be obscured by the soft keyboard

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Input, defect)

Description

[Jim Mathies [:jimm]]

STR:

1) open test case
2) scroll the page such that the form input is down where the keyboard will be displayed
3) tap the input

result: form input obscured by the keyboard

Comment 1

[Jim Mathies [:jimm]]

Attachment: testcase

Comment 2

[Jim Mathies [:jimm]]

Attachment: wip

Comment 4

[Jim Mathies [:jimm]]

Attachment: part 01 - cao and reposition info request support

Comment 5

[Jim Mathies [:jimm]]

Attachment: part 02 - dealing with DeckOffset events in selection code

Comment 6

[Jim Mathies [:jimm]]

The way this works -

1) cao gets a skb display event.
2) cao queries content for information on whether or not the focused input needs to be raised.
3) if it needs to be raised, cao sets an offset on the main browser deck hooked up to a nice transition animation.
4) when the skb is hidden, the deck offset is removed.

selection and menus (and ??) front end need to know about the shifting so they can adjust ui elements. I have a patch here for selection. I'll look at menus next which shouldn't be too hard to fix up.

One note about margin-top vs. transform - there is a bug where a transformed document doesn't work with the selectAtPoint api in domwindoutils. I'm currently using margin-top instead. (I've been fighting security restrictions trying to get a xul test case written that shows this issue. Will file a bug on it when I get this working.)

Comment 7

[Jim Mathies [:jimm]]

Attachment: part 02(a) - selection code cleanup

Comment 8

[Jim Mathies [:jimm]]

Attachment: part 02(b) - dealing with DeckOffset events in selection

Comment 9

[Jim Mathies [:jimm]]

Comment on attachment 732976 [details] [diff] [review]
part 02(a) - selection code cleanup

to avoid clutter I'm going to split selection and menus out into separate bugs.

Comment 10

[Matt Brubeck (:mbrubeck)]

Comment on attachment 732971 [details] [diff] [review]
part 01 - cao and reposition info request support

Review of attachment 732971 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/metro/base/content/contenthandlers/Content.js
@@ +282,5 @@
>      addMessageListener("Browser:SetCharset", this);
>      addMessageListener("Browser:CanUnload", this);
>      addMessageListener("Browser:PanBegin", this);
>  
> +    addMessageListener("Browser:RepositionInfoRequest", this);

Can we add this message listener in SelectionHandler.js instead, rather than "proxying" it through Content.js?

@@ +416,5 @@
> +   */
> +  _repositionInfoRequest: function _repositionInfoRequest(aJsonMsg) {
> +    if (!SelectionHandler.isActive) {
> +      Util.dumpLn("unexpected: repositionInfoRequest but selection isn't active.");
> +      sendAsyncMessage("Content:RepositionInfoRequest", { reposition: false });

Nit: Please remove "Request" from this message name.  (You can replace it with "Response" if you like -- I saw the email thread about message name convention, and am happy with whatever convention we want to encourage.)

Comment 11

[Jim Mathies [:jimm]]

(In reply to Matt Brubeck (:mbrubeck) from comment #10)
> Comment on attachment 732971 [details] [diff] [review]
> part 01 - cao and reposition info request support
> 
> Review of attachment 732971 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/metro/base/content/contenthandlers/Content.js
> @@ +282,5 @@
> >      addMessageListener("Browser:SetCharset", this);
> >      addMessageListener("Browser:CanUnload", this);
> >      addMessageListener("Browser:PanBegin", this);
> >  
> > +    addMessageListener("Browser:RepositionInfoRequest", this);
> 
> Can we add this message listener in SelectionHandler.js instead, rather than
> "proxying" it through Content.js?

I might have been too nit picky on organization, but it seemed like Content.js was the more appropriate target. A developer looking for this (assuming they didn't have mxr) would go to something like Content.js before they looked in selection code.

Comment 12

[Matt Brubeck (:mbrubeck)]

(In reply to Jim Mathies [:jimm] from comment #11)
> I might have been too nit picky on organization, but it seemed like
> Content.js was the more appropriate target. A developer looking for this
> (assuming they didn't have mxr) would go to something like Content.js before
> they looked in selection code.

Perhaps we should put the message names in a "SelectionHandler:" namespace as an additional hint, to help casual readers guess the right file.

However, grep/MXR are always necessary if you really want to know who listens to a given message or event.  And it'd be nice if SelectionHandler can stand on its own without these added dependencies on Content.js.

Comment 13

[Jim Mathies [:jimm]]

(In reply to Matt Brubeck (:mbrubeck) from comment #12)
> (In reply to Jim Mathies [:jimm] from comment #11)
> > I might have been too nit picky on organization, but it seemed like
> > Content.js was the more appropriate target. A developer looking for this
> > (assuming they didn't have mxr) would go to something like Content.js before
> > they looked in selection code.
> 
> Perhaps we should put the message names in a "SelectionHandler:" namespace
> as an additional hint, to help casual readers guess the right file.
> 
> However, grep/MXR are always necessary if you really want to know who
> listens to a given message or event.  And it'd be nice if SelectionHandler
> can stand on its own without these added dependencies on Content.js.

Ok good point. I'll move it over.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9a0aa8302b9e