599362 - Can still search in Awesome screen once the context menu is shown

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aakash Desai [:aakashd]]

Build Id:
Mozilla/5.0 (Maemo; Linux armv71; rv:2.0b6pre) Gecko/20100923 Namoroka/4.0b7pre
Fennec/2.0b1pre

and

Mozilla/5.0 (Android; Linux armv71; rv:2.0b6pre) Gecko/20100923
Namoroka/4.0b7pre Fennec/2.0b1pre

Steps to Reproduce:
1. Open the awesomescreen
2. Press down on any row entry.
3. After the context menu has popped up, type in any search term

Actual Results:
The awesome screen list completes a search

Expected results: 
Three possibilities - 
1. The soft kb is hidden when a context menu pops-up
2. The soft kb is visible, but is not functional (no search is done).
3. The soft kb is visible. When something is typed in, the context menu disappears.

Comment 1

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

UX guys - got any direction for this situation?

What do we do with the virtual keyboard when a context menu is displayed? Is the general case and the awesomebar case different?

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

We should be doing what Android does, which is to make context menus modal in terms of interaction. The user should not be able to interact with anything other than the context menu while shown, and the Android Back button should dismiss that menu.

In fact, if possible, we should be *using* the Android context menu UI, not our own thing.

Specific to this issue: the soft keyboard (and physical keyboard!) should be disabled.

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #2)
> In fact, if possible, we should be *using* the Android context menu UI, not our
> own thing.

mfinkle, if you want support for native context menus, please file a bug.

Comment 4

[Wesley Johnston (:wesj)]

For the default browser, the virtual keyboard is not dismissed when context menus appear (i.e. the menus appear over the top of the keyboard). Seems like the only way to get that behaviour is to go native.

I don't mind digging into native menu support. Do we want it? Otherwise I can just blur whatever is focused when the menu appears, and focus it again when the menu leaves.

Comment 5

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

I don't want to look at native menus for Fennec 4.0 timeframe. Maybe after we can think about it. We need to consider add-ons and extensibility. Native menus could make that more difficult. I don't want to worry about it now.

Comment 6

[Wesley Johnston (:wesj)]

Attachment: Blur focused element

This patch blurs the currently focused element. However, our urlbar also goes to some pains to take keyboard input even if it isn't focused, so I had to put another check there to make sure no popups are open. I'm not sure if a more specific check (no modal popups?) is necessary or not.

I also switched out emptytext attributes to use placeholder instead.

Comment 7

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

Comment on attachment 482562 [details] [diff] [review]
Blur focused element

I worry about the blurFocusedElement messing up focus after the contextmenu is dismissed. We should leave the same element focused, right?

Comment 8

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

(In reply to comment #7)
> Comment on attachment 482562 [details] [diff] [review]
> Blur focused element
> 
> I worry about the blurFocusedElement messing up focus after the contextmenu is
> dismissed. We should leave the same element focused, right?

Doing that also means we would dismiss the VKB if the user bring up the popupmenu on an awesome result. Is it what we want?

Comment 9

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

(In reply to comment #8)
> (In reply to comment #7)
> > Comment on attachment 482562 [details] [diff] [review] [details]
> > Blur focused element
> > 
> > I worry about the blurFocusedElement messing up focus after the contextmenu is
> > dismissed. We should leave the same element focused, right?
> 
> Doing that also means we would dismiss the VKB if the user bring up the
> popupmenu on an awesome result. Is it what we want?

I think so. Without hiding the VKB we would not be able to see the context menu or popup.

Comment 10

[Wesley Johnston (:wesj)]

> I worry about the blurFocusedElement messing up focus after the contextmenu is
> dismissed. We should leave the same element focused, right?

I did that first, but the simple move (just calling .focus()) will select all the text in the urlbar. I figured that I would start the simplest solution.

Trying to restore the state perfectly in all situations will take some work. We can just call focus and maybe restore the selection if its a text input though?

Comment 11

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

Comment on attachment 482562 [details] [diff] [review]
Blur focused element

>diff --git a/chrome/content/browser-ui.js b/chrome/content/browser-
>-        if (this.activePanel && this._edit.readOnly)
>+        if (this.activePanel && this._edit.readOnly && !this._popup)
>           this._edit.readOnly = false;

You should not need to worry about this anymore.

>     this._panel.hidden = false;
>+    BrowserUI.blurFocusedElement();
>     window.addEventListener("resize", this, true);

This is probably all you need, I don't think we need to care about restoring the focus to the previously selected element for now.

Please rebased your patch and I'll r? it.

Comment 12

[Wesley Johnston (:wesj)]

Attachment: Patch 2

Patch v2. Actually, I was looking at popup code this morning and saw these sections where they store and restore focus:

http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/popup.xml#227

That made me feel more comfortable with the idea of restoring focus if we want to do it.

Comment 13

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

http://hg.mozilla.org/mobile-browser/rev/bf3697b9ce24

Comment 14

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

verified:
Mozilla/5.0 (Maemo;Linux armv71; rv:2.0b8pre) Gecko/20101019 Firefox/4.0b8pre Fennec/4.0b2pre
Mozilla/5.0 (Android; Linux armv71; rv2.0b8pre) Gecko/20101019 Firefox/4.0b8pre Fennec/4.0b2pre