Closed Bug 684558 Opened 9 years ago Closed 9 years ago

Android back button goes back twice

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 10

People

(Reporter: mbrubeck, Assigned: mbrubeck)

References

Details

(Keywords: regression, Whiteboard: [pushed])

Attachments

(2 files, 2 obsolete files)

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?
tracking-fennec: --- → ?
This is also reproducible on desktop with the "escape" key.
OS: Android → All
Hardware: ARM → All
matt, can you try backing out each of those patches locally to see if that fixes the problem?
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.
i think you or oleg must back it out now, and require a test when those other two patches land.
Attached patch patch (obsolete) — Splinter Review
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.
Attachment #558191 - Flags: review?(mark.finkle)
changeset:   76524:91d411d25d76
user:        Doug Turner <dougt@dougt.org>
date:        Sun Sep 04 13:08:49 2011 -0700
summary:     Backing out d4c725db7369 (bug 683736).

changeset:   76525:a4a584a110e0
tag:         tip
user:        Doug Turner <dougt@dougt.org>
date:        Sun Sep 04 13:11:42 2011 -0700
summary:     Backing out bc4b2d84f80a (bug 682017)

backed out of m-c.
Attachment #558191 - Flags: review?(mark.finkle) → review+
Attached patch test (obsolete) — Splinter Review
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.
tracking-fennec: ? → 9+
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.
Status: NEW → ASSIGNED
Attached patch test v2Splinter Review
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.
Attachment #559854 - Attachment is obsolete: true
Attachment #563533 - Flags: review?(mark.finkle)
Comment on attachment 563533 [details] [diff] [review]
test v2

Do you need the sendEscape() function in the test file?
Attachment #563533 - Flags: review?(mark.finkle) → review+
(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
Whiteboard: [pushed to try]
The Try run showed a new timeout in existing test browser_awesomescreen.js; probably just need to update some event generation in the test.
Whiteboard: [pushed to try] → [needs an additional test fix]
Attached patch patch v2Splinter Review
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.
Attachment #558191 - Attachment is obsolete: true
Attachment #563836 - Flags: review+
https://tbpl.mozilla.org/?tree=Try&rev=654c636e0207
Whiteboard: [needs an additional test fix] → [pushed to try]
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
tracking-fennec: 9+ → ---
Whiteboard: [pushed to try] → [pushed]
Target Milestone: --- → Firefox 10
Version: Firefox 9 → Trunk
Mozilla/5.0 (Android; Linux armv7l; rv:10.0a1) Gecko/20111003 Firefox/10.0a1 Fennec/10.0a1
Status: RESOLVED → VERIFIED
Depends on: 691175
Blocks: 691418
You need to log in before you can comment on or make changes to this bug.