320203 - Respect the Firefox/SeaMonkey pref for browser.backspace_action

Status: VERIFIED FIXED

(Camino Graveyard :: Accessibility, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Gecko has a pref to control what happens when you hit backspace key when a textarea is not focused (go back, page down, nothing).  Camino doesn't respect this pref, and it should.

This *should* be an easy way to once and for all end the contention between the "go back" faction and the "do nothing" faction ;)

Comment 1

[Chris Lawson (gone)]

We're going to need to patch one of the prefs files, but I'm not sure how we want to handle it. My initial instinct is to patch

http://lxr.mozilla.org/mozilla/source/camino/resources/application/all-camino.js

but the other option would be

http://lxr.mozilla.org/mozilla/source/xpfe/bootstrap/browser-prefs.js

Otherwise, I should be able to get this taken care of shortly.

cl

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Don't we have to remove the hard-coded behavior, too?

I'd reckon we put the default (i.e., backspace goes back) in all-camino.js.

Comment 3

[Chris Lawson (gone)]

(In reply to comment #2)
> Don't we have to remove the hard-coded behavior, too?

Yeah. I've got it patched for the 0 and >1 cases, but not for 1 (PgDn/PgUp) yet. The hardcoded behaviour just needed a quick if statement added to check the pref.
 
> I'd reckon we put the default (i.e., backspace goes back) in all-camino.js.

Yeah, that's what Mark told me on IRC earlier.

cl

Comment 4

[Chris Lawson (gone)]

Attachment: Patches the 0 and >1 cases

This doesn't fully support the pref (it doesn't do anything for pref = 1, which should map to PgUp/PgDn) but Simon said to upload and then decide whether or not we care about supporting that action in the first place.

If anyone cc'd on this bug knows what NSView I should be passing the scroll event to, please comment here.

cl

Comment 5

[Stuart Morgan]

Is there any value to supporting a Linux setting which has absolutely no precedent on the platform?  Every request that's come in has just been for a way to turn it off, which is possible without supporting the '1' setting.

Comment 6

[Chris Lawson (gone)]

Attachment: Includes all-camino.js, no support for PgUp/PgDn

Per fairly thorough discussion on IRC, not gonna worry about supporting the Gnome mappings at all. This keeps things a lot simpler and solves the problem for 100% of our complainants (including me).

This patch adds in the all-camino.js modification as well.

cl

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

*** Bug 328034 has been marked as a duplicate of this bug. ***

Comment 8

[Stuart Morgan]

Comment on attachment 209548 [details] [diff] [review]
Includes all-camino.js, no support for PgUp/PgDn

This patch is malformed. Given that it has a net increase of 6 lines, but thinks it should add 14:
@@ -2484,29 +2484,43 @@
I'm guessing you edited it by hand--please don't ever post hand-edited patches without verifying that they still apply.

Also, it looks like you made two different patches in two different directories, then stuck them together, which makes it even harder to apply. It's easier on reviewers if you make all patches from either the mozilla directory or the camino directory, but you definitely need to make multi-file patches from at least the nearest common ancestor directory.

Comment 9

[Chris Lawson (gone)]

Attachment: Fixed, ready for checkin

Fixed and respun without any hand-editing.

cl

Comment 10

[Stuart Morgan]

Comment on attachment 213273 [details] [diff] [review]
Fixed, ready for checkin

r=me

Comment 11

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 213273 [details] [diff] [review]
Fixed, ready for checkin

sr=pink

Comment 12

[Håkan Waara]

Fix checked into trunk for cl.

Comment 13

[Håkan Waara]

Checked into the 1.8-branch as well.

Comment 14

[Asaf Romano (gone)]

Not a Gecko thing ;)

Comment 15

[Chris Lawson (gone)]

Verified since I was pointing this out to someone on feedback anyway; this definitely works now...