Focused form inputs can be obscured by the soft keyboard

RESOLVED FIXED in Firefox 23

Status

Firefox for Metro
Input
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jimm, Assigned: jimm)

Tracking

Trunk
Firefox 23
x86_64
Windows 8.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

5 years ago
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
(Assignee)

Comment 1

5 years ago
Created attachment 730230 [details]
testcase
(Assignee)

Updated

5 years ago
Depends on: 856008
(Assignee)

Comment 2

5 years ago
Created attachment 732291 [details] [diff] [review]
wip
Assignee: nobody → jmathies
(Assignee)

Updated

5 years ago
Duplicate of this bug: 855235
(Assignee)

Updated

5 years ago
Attachment #732291 - Attachment is obsolete: true
(Assignee)

Comment 4

5 years ago
Created attachment 732971 [details] [diff] [review]
part 01 - cao and reposition info request support
(Assignee)

Comment 5

5 years ago
Created attachment 732973 [details] [diff] [review]
part 02 - dealing with DeckOffset events in selection code
(Assignee)

Comment 6

5 years ago
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.)
(Assignee)

Updated

5 years ago
Attachment #732971 - Flags: review?(mbrubeck)
(Assignee)

Comment 7

5 years ago
Created attachment 732976 [details] [diff] [review]
part 02(a) - selection code cleanup
Attachment #732973 - Attachment is obsolete: true
(Assignee)

Comment 8

5 years ago
Created attachment 732977 [details] [diff] [review]
part 02(b) - dealing with DeckOffset events in selection
(Assignee)

Comment 9

5 years ago
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.
Attachment #732976 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #732977 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
No longer blocks: 850413
(Assignee)

Updated

5 years ago
Blocks: 857823
(Assignee)

Updated

5 years ago
Blocks: 857825
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.)
Attachment #732971 - Flags: review?(mbrubeck) → review+
(Assignee)

Comment 11

5 years ago
(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.
(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.
(Assignee)

Comment 13

5 years ago
(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.
https://hg.mozilla.org/mozilla-central/rev/9a0aa8302b9e
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.