444892 - when find bar is focused, ctrl+pageup/dn will scroll page content

Status: VERIFIED FIXED

(Toolkit :: Find Toolbar, defect)

Description

[Ori Avtalion (salty-horse)]

Tested on Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9) Gecko/2008061017 Firefox/3.0

When the find bar is focused, ctrl+pageup/down will send the pageup/down action to the current page before switching tabs.

It should *not* scroll the page when ctrl is pressed.

Steps to reproduce:
1) Open two tabs with lots of vertical scroll. Move the scroll to the middle of the page.
2) Open a find bar in the first tab (will automatically focus) and use ctrl+pagedown to switch to the second tab.
3) Return to the first tab with ctrl+pageup

Results:
1) The page on the first tab has scrolled down. (since the find-bar was focused)
2) The page on the second tab did not scroll up (since the find-bar was not focused

Comment 1

[Jesse Ruderman]

http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/findbar.xml needs to check for keyboard modifiers before scrolling.

Comment 2

[Ori Avtalion (salty-horse)]

Jesse, is CTRL the only "dangerous" modifier? Can you think of others that should be checked for?

Comment 3

[Jesse Ruderman]

I think it should bail if any of cmd/meta, option/alt, or ctrl are held.  Maybe even shift.

Comment 4

[Ori Avtalion (salty-horse)]

Attachment: Suggested patch

Attached is a patch that prevents pageup/down from scrolling the page when pressed in conjunction with ctrl/alt/shift.

I'm not sure if event.preventDefault() should be inside or outside the conditional.

Does shift need to be checked as well?

Comment 5

[Neil Deakin]

Comment on attachment 364815 [details] [diff] [review]
Suggested patch

I think shift should be ignored as well.

Comment 6

[Ori Avtalion (salty-horse)]

Attachment: Added shift to the list of keys to ignore

Ok. The new patch also ignores shift.

Comment 7

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/306d51ee8b10

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 368030 [details] [diff] [review]
Added shift to the list of keys to ignore

a191=beltzner

Comment 9

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c700c9b5c74e

Comment 10

[Aakash Desai [:aakashd]]

verified FIXED on builds: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090505 Minefield/3.6a1pre ID:20090505031205

and

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b5pre) Gecko/20090505 Shiretoko/3.5b5pre ID:20090505031155

Comment 11

[Alex Faaborg [:faaborg] (Firefox UX)]

This bug's priority relative to the set of other polish bugs is:
P3 - Polish issue that is in a secondary interface, occasionally encountered, or is not easily identifiable.