Closed Bug 599362 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set

Tracking

(fennec2.0+)

VERIFIED FIXED
Tracking Status
fennec 2.0+ ---

People

(Reporter: aakashd, Assigned: wesj)

Details

(Whiteboard: [fennec-checkin-postb2])

Attachments

(1 file, 1 obsolete file)

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.
tracking-fennec: --- → ?
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?
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.
tracking-fennec: ? → 2.0+
(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.
Assignee: nobody → wjohnston
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.
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.
Attached patch Blur focused element (obsolete) — Splinter Review
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.
Attachment #482562 - Flags: review?(mark.finkle)
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?
Attachment #482562 - Flags: review?(21)
(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?
(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.
> 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 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.
Attached patch Patch 2Splinter Review
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.
Attachment #482562 - Attachment is obsolete: true
Attachment #483365 - Flags: review?(21)
Attachment #482562 - Flags: review?(mark.finkle)
Attachment #482562 - Flags: review?(21)
Whiteboard: [fennec-checkin-postb2]
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.