691175 - Unable to back out of Awesome screen

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

Pressing my devices back button will not back out of the awesome screen; a recent regression.

STR:
1. New profile, start Fennec
2. Tap the Awesomebar
3. Tap your devices back button twice

ER: Back looking at about:home
AR: Stuck in the Awesome screen

--
Mozilla/5.0 (Android; Linux armv7l; rv:10.0a1) Gecko/20111002 Firefox/10.0a1 Fennec/10.0a1

Samsung Galaxy SII
Samsung Nexus S

Comment 1

[Aaron Train [:aaronmt]]

Under the tablet UI, the panel remains open but one can tap outside of it to dismiss it

--
Mozilla/5.0 (Android; Linux armv7l; rv:10.0a1) Gecko/20111003 Firefox/10.0a1 Fennec/10.0a1

Samsung Galaxy Tab 10.1

Comment 2

[Aaron Train [:aaronmt]]

First known bad:

20111002030844
http://hg.mozilla.org/mozilla-central/rev/90575e23ea93

Last known Good:
20111001030845
http://hg.mozilla.org/mozilla-central/rev/f25928e4847d

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f25928e4847d&tochange=90575e23ea93

Possible culprits?
http://hg.mozilla.org/mozilla-central/rev/95b7fab4037c
http://hg.mozilla.org/mozilla-central/rev/357f8c513c65
http://hg.mozilla.org/mozilla-central/rev/a89bf0c36977

Comment 3

[Matt Brubeck (:mbrubeck)]

Regression from bug 684558.  Specifically, this broke because I changed the BrowserUI "keypress" handler from capture=true to capture=false.

Comment 4

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This fixes the bug, and does not regress bug 684558 or bug 683736.

This also backs out a change I made to browser_awesomescreen.js that I think caught this bug but I thought it was a bug in the test.  Oops.  :(  Tests are passing on desktop except for browser_awesomescreen which is crashing on my desktop build for some unrelated reason (with or without this patch).  Pushed to try to test on device:
https://tbpl.mozilla.org/?tree=Try&rev=f09d8a58271b

It feels kind of hacky that we have to do this.  I'd like to file a followup bug on making the platform side of this better so that we don't need to do our own event re-dispatching (though I don't know exactly how that would work).  But this is no hackier than the code that was removed by bug 682017, so I think the best way forward is to stick with the platform event forwarding, fix this bug, and try to clean up the platform to eventually remove the need for these hacks.  (The alternative is to back out bug 682017 and all its dependencies and just go back to doing all the forwarding with JavaScript messages.)

Comment 5

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

Comment on attachment 564230 [details] [diff] [review]
patch


>       case "keypress":
>+        // Ignore events headed toward the browser; they will be
>+        // re-dispatched after content has a chance to handle them.
>+        if (aEvent.target == Browser.selectedBrowser)
>+          break;

I suppose keypresses would only be targeted to the selected browser and not any browser. If we think it could be any browser, maybe use | aEvent.target.localName == "browser" | check instead

Comment 6

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/2c0eb6a0e6e7

Comment 7

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/2c0eb6a0e6e7

Comment 8

[Carla Nadastean]

Retested bug with:
Mozilla/5.0 (Android; Linux armv7l; rv:10.0a1) Gecko/20111004 Firefox/10.0a1 Fennec/10.0a1

Devices: 
Samsung Nexus S (Android 2.3)
LG Optimus 2X (Android 2.2)

Bug is no longer reproducible. Back button is working properly after Awesome screen is displayed.

Verifying bug.