684558 - Android back button goes back twice

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Matt Brubeck (:mbrubeck)]

Steps to reproduce:
1. In a Fennec tab, load at least three pages in succession.
2. Press the Android system "back" key

Expected results: Fennec goes back to the previous page.

Actual results: Fennec goes back twice: first to the previous page, then immediately back to the page before that.

Found in 2011-09-03 mozilla-central nightly on Motorola Xoom (Android 3.2).  Possible regression from bug 683736 or bug 683736?

Comment 1

[Matt Brubeck (:mbrubeck)]

This is also reproducible on desktop with the "escape" key.

Comment 2

[Doug Turner (:dougt)]

matt, can you try backing out each of those patches locally to see if that fixes the problem?

Comment 3

[Matt Brubeck (:mbrubeck)]

Backing out bug 683736 alone did not fix the problem, but backing out bug 682017 (plus bug 683736 which depends on it) does fix the problem.  We could back out those patches until this problem is resolved, unless there is an easy fix.

Comment 4

[Doug Turner (:dougt)]

i think you or oleg must back it out now, and require a test when those other two patches land.

Comment 5

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This patch fixes the problem, and on Tuesday I will write a test for escape key behavior.  I think it's fine if we want to back the previous patches out until this one and the test are ready to land.

Comment 6

[Doug Turner (:dougt)]

changeset:   76524:91d411d25d76
user:        Doug Turner <[e-mail]>
date:        Sun Sep 04 13:08:49 2011 -0700
summary:     Backing out d4c725db7369 (bug 683736).

changeset:   76525:a4a584a110e0
tag:         tip
user:        Doug Turner <[e-mail]>
date:        Sun Sep 04 13:11:42 2011 -0700
summary:     Backing out bc4b2d84f80a (bug 682017)

backed out of m-c.

Comment 7

[Matt Brubeck (:mbrubeck)]

Attachment: test

This adds a couple of tests for the escape key, but this bug doesn't actually cause the test to fail.  It seems that events sent through EventUtils.sendKey (which uses element.dispatchEvent) or EventUtils.synthesizeKey (which uses nsIDOMWindowUtils.sendKeyEvent) do not trigger the buggy behavior, but real keyboard events do.  Still trying to figure this out.

Comment 8

[Gabriela [:gaby2300]]

Build ID: Mozilla/5.0 (Android; Linux armv7l; rv:9.0a 1)Gecko/20110922 Firefox/9.0a1 Fennec/9.0a1
SO: Android 3.1
Device: Samsung Galaxy Tab 10.1

I cannot reproduce this bug.

Comment 9

[Matt Brubeck (:mbrubeck)]

Attachment: test v2

Unfortunately this test case does not catch the exact regression that this bug fixes, but it does test the basics and catches some other possible regressions (including ones I actually introduced while trying to develop the test).

I couldn't find a way to catch the "back twice" regression without using large timeouts.  :(   I'd like to play with this some more, but for now I think we can re-land the patches with this regression fix and test.

Comment 10

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

Comment on attachment 563533 [details] [diff] [review]
test v2

Do you need the sendEscape() function in the test file?

Comment 11

[Matt Brubeck (:mbrubeck)]

(In reply to Mark Finkle (:mfinkle) from comment #10)
> Do you need the sendEscape() function in the test file?

No, that was left over accidentally from a previous version.  Removed.

Pushed to Try: https://tbpl.mozilla.org/?tree=Try&rev=9a541c98d546

Comment 12

[Matt Brubeck (:mbrubeck)]

The Try run showed a new timeout in existing test browser_awesomescreen.js; probably just need to update some event generation in the test.

Comment 13

[Matt Brubeck (:mbrubeck)]

Attachment: patch v2

Simple test-only change to fix the timeout in browser_awesomescreen.js.  Carrying r=mfinkle.  Pushing this to Try again to see results on Tegra.

Comment 14

[Matt Brubeck (:mbrubeck)]

https://tbpl.mozilla.org/?tree=Try&rev=654c636e0207

Comment 15

[Matt Brubeck (:mbrubeck)]

Removing tracking-fennec:9+ because the bug that caused this was backed out of Fennec 9.

New Try run is green: https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=cdb8f52e8801

Pushed to inbound along with dependent patches:
https://hg.mozilla.org/integration/mozilla-inbound/rev/357f8c513c65
https://hg.mozilla.org/integration/mozilla-inbound/rev/95b7fab4037c

Comment 16

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/357f8c513c65
https://hg.mozilla.org/mozilla-central/rev/95b7fab4037c

Comment 17

[Aaron Train [:aaronmt]]

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