Closed Bug 400568 Opened 13 years ago Closed 11 years ago

Keyboard Back/Forward keys sending VK_BROWSER_BACK/FORWARD go back/forward twice

Categories

(Core :: Widget: Win32, defect, minor)

x86
Windows XP
defect
Not set
minor

Tracking

()

RESOLVED FIXED

People

(Reporter: bugs1m.wisefool, Assigned: mayhemer)

References

Details

(Keywords: fixed1.9.1)

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8) Gecko/2007091216 GranParadiso/3.0a8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8) Gecko/2007091216 GranParadiso/3.0a8

This is a regression I'm noticing on my Thinkpad T60, Windows XP laptop. If I click on a fragment identifier for the page I'm on currently, pressing Thinkpad's Back key will jump back two URL's instead of one. Alt+left still goes back one as expected. The Forward key has the same problem.

Reproducible: Always

Steps to Reproduce:
1. Go to about:
2. Go to http://en.wikipedia.org/wiki/Mozilla_Firefox
3. Click 'History' in table of contents
4. Press 'Back' button on Thinkpad keyboard
Actual Results:  
about: is displayed

Expected Results:  
It should behave the same as if I pressed the Back button in the UI
Version: unspecified → Trunk
I can't confirm this for you, I'm afraid. Perhaps try it with the latest build first.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007102005 Minefield/3.0a9pre ID:2007102005
I just checked; it still happens with a fresh profile in the 2007-10-21 nightly.
Do you happen to know when this started regressing?
Alright, I just tested it; this happens in the 2006-12-01-04 nightly, but doesn't happen in the 2006-11-30-04 nightly or Gran Paradiso Alpha 1.
I'm still experiencing this. Is there any information in my system configuration I can ferret out that would be useful? I'm not sure if the back/forward keys are interpreted by Windows XP, or a Thinkpad driver that might be the problem.

Again, this first regressed in the 2006-12-01-04 nightly.
I could not reproduce this with FF 3.0.
Status: UNCONFIRMED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
(In reply to comment #6)
> I could not reproduce this with FF 3.0.
> 

The reporter wrote this bug still happens to him. Furthermore the correct resolution should be at least worksforme :-) I reopen it.

Wisefool, does this bug happens to you with the latest trunk (3.1) with a blank profile?
Status: RESOLVED → UNCONFIRMED
Resolution: FIXED → ---
I have a Thinkpad T43p and used to experience this bug, then I updated a bunch of Thinkpad drivers and the problem disappeared.  Apparently Lenovo has squashed this one for us.
I was testing with quit new T61 and could not reproduce this problem.

In reply to comment 8: could you please just post a comment for record where/how did you update the drivers if it were not updated automatically by Lenovo update software? It could be useful for others still experiencing the problem and looking for solution.

Thanks.
Actually, the driver update does not seem to have entirely corrected the problem.  Now it seems to do it only some of the time.  Once it starts, it will consistently do it while Firefox remains open, but I can't always force it to reproduce the bug.
After additional testing...

I have managed to encounter the bug on a non-Thinkpad.  Apparently it is caused by the generic "Back" and "Forward" keyboard codes, as I have a pretty generic USB keyboard that didn't come with any driver, and using its Back and Forward buttons produce the problem.  Presumably, these are the VK_BROWSER_BACK and VK_BROWSER_FORWARD keycodes in Windows.

Also, it only occurs when the current location and the previous (or next, in the case of forward) location are different parts of the same page.  The page you end up on (+/- 2 locations) doesn't seem to matter.

Hopefully this information will help others reproduce the bug.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208

Confirming. One further observation: When the issue happens, the load state of the jumped over page and the one following it isn't "Done" but "Stopped".
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Thinkpad Back/Forward keys go back/forward twice → Keyboard Back/Forward keys sending VK_BROWSER_BACK/FORWARD go back/forward twice
Duplicate of this bug: 452359
qgl6049: thanks, I can reproduce this. Assigning to me.
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Attached patch v1 (obsolete) — Splinter Review
Problem is that we receive two events: WM_APPCOMMAND with lParam=APPCOMMAND_BROWSER_BACKWARD/FORWARD which is handled from PeekKeyAndIMEMessage and event WK_(SYS)KEYDOWN with code VK_BROWSER_BACK/FORWARD handled later in the same cycle. Both these events lead to call of nsSHistory::GoBack/GoForward which goes exactly to history item at nsSHistory::mIndex+1 or -1. In case of anchor navigation (moving on the same page) mIndex indicating on what item of the history list we currently are is updated synchronously (call to nsSHistory::UpdateIndex), before the second call to nsSHistory::GoBack/GoForward function. We actually always duplicate its call when using these back/forward keys but it is ignored because usually mIndex is not updated that soon and leads just to reload fo the same page, invisible to user.

The patch therefor ignores the key events because it seems that system always indicates its press by application command event type as well. I have no chance to test this with a mouse having back/forward buttons nor with such a keyboard.
Attachment #348688 - Flags: review?(emaijala)
Flags: wanted1.9.0.x?
Flags: wanted-firefox3.1?
Or perhaps OnKeyDown shouldn't just call DispatchCommandEvent at all?
So basically removing #ifdef VK_BROWSER_BACK...#endif.
(I don't have windows+right kind of keyboard to test)
Attachment #348688 - Flags: review?(emaijala) → review-
Comment on attachment 348688 [details] [diff] [review]
v1

I believe Smaug is right, though it seems the only way to be sure is remove the whole block, check it in and hope nothing regresses. I don't think we need to prevent the keys from being dispatched, or then we should do that consistently to all such keys.
Attached patch v2Splinter Review
This patch removes the special handling of back and forward keys from both OnKeyDown/Up. It seems to work the same way as the previos patch.
Attachment #348688 - Attachment is obsolete: true
Attachment #348826 - Flags: review?(emaijala)
Attachment #348826 - Flags: review?(emaijala) → review+
I just posted the patch to TryServer so that anyone with right kind of
keyboard can test the patch even before it lands trunk.
https://build.mozilla.org/tryserver-builds/2008-11-19_03:05-opettay@mozilla.com-win_b_f/

The Windows build isn't ready yet.
Attachment #348826 - Flags: superreview?(roc)
Attachment #348826 - Flags: superreview?(roc) → superreview+
Attachment #348826 - Flags: approval1.9.0.5?
Comment on attachment 348826 [details] [diff] [review]
v2

Pushing out nomination since 1.9.0.5 is basically code frozen.
Attachment #348826 - Flags: approval1.9.0.5? → approval1.9.0.6?
Attachment #348826 - Flags: approval1.9.1?
(In reply to comment #19)
> I just posted the patch to TryServer so that anyone with right kind of
> keyboard can test the patch even before it lands trunk.
> https://build.mozilla.org/tryserver-builds/2008-11-19_03:05-opettay@mozilla.com-win_b_f/
> 
> The Windows build isn't ready yet.

Did anyone do this? Do we know that this patch works?
Attachment #348826 - Flags: approval1.9.1?
Attachment #348826 - Flags: approval1.9.1-
Attachment #348826 - Flags: approval1.9.0.6?
Attachment #348826 - Flags: approval1.9.0.6-
Comment on attachment 348826 [details] [diff] [review]
v2

Not approved until someone answers my question in the previous comment; feel free to renominate once that's done.
Flags: wanted1.9.0.x? → wanted1.9.0.x+
Component: Keyboard Navigation → Widget: Win32
Flags: wanted-firefox3.1?
Product: Firefox → Core
QA Contact: keyboard.navigation → win32
Attachment #348826 - Flags: approval1.9.1?
Attachment #348826 - Flags: approval1.9.1-
Attachment #348826 - Flags: approval1.9.0.6?
Attachment #348826 - Flags: approval1.9.0.6-
Comment on attachment 348826 [details] [diff] [review]
v2

Just confirmed with testing build and keyboard with BACK/FORWARD keys: FF 3.0.5 - skips by two, testing build - skips by one. Renominating, unfortunately too late for 3.0.6.
Attachment #348826 - Flags: approval1.9.0.6? → approval1.9.0.7?
Comment on attachment 348826 [details] [diff] [review]
v2

Moving nomination to 1.9.0.7.
Why hasn't this landed on trunk yet?
(In reply to comment #25)
> Why hasn't this landed on trunk yet?

Landed on trunk as http://hg.mozilla.org/mozilla-central/rev/19757a81c07c.
(In reply to comment #26)
> Landed on trunk as http://hg.mozilla.org/mozilla-central/rev/19757a81c07c.

RESOLVED FIXED, then.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago11 years ago
Resolution: --- → FIXED
Whiteboard: [needs 1.9.1 approval, baking]
Comment on attachment 348826 [details] [diff] [review]
v2

We'll wait until after 1.9.1 to approve.
Attachment #348826 - Flags: approval1.9.0.7?
Comment on attachment 348826 [details] [diff] [review]
v2

a191=beltzner

Can we get a followup test for this (and perhaps other keybindings as well) please?
Attachment #348826 - Flags: approval1.9.1? → approval1.9.1+
This needs a litmus test, afaik.
Flags: in-litmus?
I don't think I can create an automated test for this. If this needs a litmus test, please add it someone, I wasn't able to find a way to add a new test. I can provide exact and simple STR if needed.
Whiteboard: [needs 1.9.1 approval, baking] → [needs 1.9.1 landing]
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8effb4f50f25
Keywords: fixed1.9.1
Whiteboard: [needs 1.9.1 landing]
I'm getting this on my Thinkpad T60P, Windows XP laptop, on Google search results.

Small note: Strangely, it works fine on google.com/firefox.
Okay, it's good in the latest trunk.
I am running 3.0.11.
Flags: in-litmus?
You need to log in before you can comment on or make changes to this bug.