591307 - Hardware and soft keyboard stop working after displaying a dialog in a new window

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Ioana Chiorean]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100826 Minefield/4.0b5pre
Build Identifier: Mozilla /5.0 (Android;Linux armv7l; rv:2.0b5pre)Gecko/20100826 Namoroka/4.0b5pre Fennec /2.0b1pre

trying top save the banner from http://slickdeals.net/ will open the system save image dialog which till now is not working right. Still the cancel button exists the system save image dialog, so this is should exit normally and when you press the URL bar you should be able to use the SW keyboard. 

Reproducible: Always

Steps to Reproduce:
1. Go to http://slickdeals.net/
2. Tap and hold down on the Slickdeals banner image link on the top left side of the page
3. Select "Save Image"
4. On the system save image dialog, press cancel button 
5. Tap the URL bar
Actual Results:  
the SW keyboard doesn't pop up, is not available

Expected Results:  
the SW keyboard pops up, is available is available to use it

Comment 1

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

Bug 590779 added code so that it requires 2 taps on the URLbar for the soft keyboard to appear. Does that work for you?

Comment 2

[Ioana Chiorean]

no, unfortunately even if i tap more than once, the SW keyboard doesn't appear

Comment 3

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

I'm seeing that too with other images.

Comment 4

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

I'm thinking of a problem with the current focused element when coming back from the "Save as" dialog because hardware buttons seems to have no effect on the Fennec UI anymore after these steps to reproduce. (Except the Home button obviously)

Comment 5

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

Attachment: Patch (m-c)

The activate state of the window should be turned on when the window is show again.

So the patch makes it works but I think the "Save as" window really need some love because with this current UI you can't create a New Dir, trying to access the Home folder give you a "You don't have the permission..."

Comment 6

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

Comment on attachment 470432 [details] [diff] [review]
Patch (m-c)

2 lines patch to let the window goes back to an active state after have dismissed the "Save As" window

Comment 7

[Michael Wu [:mwu]]

Comment on attachment 470432 [details] [diff] [review]
Patch (m-c)

Sorry for taking so long to review - didn't realize this was in my queue!

Can you try putting something like if (!gTopLevelWindows.IsEmpty()) FindTopLevel()->BringToFront(); in the nsWindow destructor and see if that works?

Comment 8

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

(In reply to comment #7)
> Comment on attachment 470432 [details] [diff] [review]
> Patch (m-c)
> 
> Sorry for taking so long to review - didn't realize this was in my queue!
> 
> Can you try putting something like if (!gTopLevelWindows.IsEmpty())
> FindTopLevel()->BringToFront(); in the nsWindow destructor and see if that
> works?

No worries, it was not urgent.
I'll try that in the next few days, thanks for the advice.

Comment 9

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

We are removing "Save As" dialogs from the application, but as a core widget issue, this bug seems like something that should be fixed. I'm not sure how you will be able to trigger the bug. You might need to add a Save As dialog back into the app.

Comment 10

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

This bug was out of my sight for a while since we've removed the "Save As" dialog but hopefully I can reproduce it by clicking on the Site Menu -> Share Page.
If you dismiss this native dialog and try to click on the urlbar the VKB won't come.

(In reply to comment #7)
> Can you try putting something like if (!gTopLevelWindows.IsEmpty())
> FindTopLevel()->BringToFront(); in the nsWindow destructor and see if that
> works?

It does not work, I assume this is because the application/window we are coming from is a native one and does not fire this code.

I've retry my patch and it works for this case

Comment 11

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

Comment on attachment 470432 [details] [diff] [review]
Patch (m-c)

Requesting review again

Comment 12

[Matt Brubeck (:mbrubeck)]

I couldn't reproduce this bug using the sharing dialog, but this patch does fix the keyboard after the non-mobile SSL dialog appears (bug 566478).  Re-nominating because we have seen this bug appear it at least two more cases besides the original one that was removed.

This bug doesn't just affect the software keyboard.  Hardware keyboards and the hardware buttons like "Back" and "Menu" also stop working when a dialog appears.  Re-summarizing to be more general.

Comment 13

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

Steps to reproduce:
 * Go to the console in the right panel
 * typed in the js eval textbox: var w = window.open("browser.xul"); w.close();
 * validate the input
 * try to click into an input field

Actual result:
 * No VKB

Expected result:
 * a VKB

Comment 14

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

Comment on attachment 470432 [details] [diff] [review]
Patch (m-c)

hmm, actually I'm not sure the patch fix the STR...

Comment 15

[Matt Brubeck (:mbrubeck)]

This bug also affects the LastPass Mobile add-on.

Comment 16

[Tony Chung [:tchung]]

can we get this landed for beta 3 if there's a patch?   if so, please approve for b3 blocker

Comment 17

[Stuart Parmenter]

Crowder, can you take a look at this?

Comment 18

[Michael Wu [:mwu]]

Attachment: Use per top-level window focus tracking

This switches to tracking the focused window per top level window instead of a single pointer to track the focused window. This is done because we need to know which window within a top level window hierarchy to send events to when switching windows, which can't be done with a single pointer. It also brings the next window up if the current top window is hidden.

Comment 19

[Doug Turner (:dougt)]

Comment on attachment 503371 [details] [diff] [review]
Use per top-level window focus tracking

sorry for the delay.  this patch has bitrotted quite a bit.  would you mind posting a new version.

Comment 20

[Michael Wu [:mwu]]

Attachment: Use per top-level window focus tracking, v2

Unbitrotted

Comment 21

[Doug Turner (:dougt)]

Comment on attachment 510397 [details] [diff] [review]
Use per top-level window focus tracking, v2

been running for a while without any noticeable issues.

Comment 22

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/0a7448775fa0

Comment 24

[Ioana Chiorean]

It seems I can't get anymore the old certification error nor the File picker is present.. Please let me know if there is a way to generate them to verify the bug or should we set this Verified as the error is not present anymore?

Comment 25

[Michael Wu [:mwu]]

(In reply to comment #24)
> It seems I can't get anymore the old certification error nor the File picker is
> present.. Please let me know if there is a way to generate them to verify the
> bug or should we set this Verified as the error is not present anymore?

Anyone at the mv office can verify this bug. Simply connect to mozillaguest and start fennec. A cert error should appear. If not, go to the pref panel and go to the addons section.

Comment 26

[Ioana Chiorean]

can you please state me a person to talk to ?

Comment 27

[Cristian Nicolae (:xti)]

Save Image feature looks different now on the latest Nightly build and it's working as expected. I will mark this bug as verified fixed.

--
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110919
Firefox/9.0a1 Fennec/9.0a1
Device: Samsung Galaxy S
OS: Android 2.2