Closed Bug 1105101 Opened 9 years ago Closed 9 years ago

OS X CMD+Up and CMD+Down arrow key shortcuts no longer scroll web page

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37
Tracking Status
firefox35 --- unaffected
firefox36 + fixed
firefox37 --- fixed

People

(Reporter: cpeterson, Assigned: athena)

References

Details

(Keywords: dogfood, regression)

Attachments

(1 file, 2 obsolete files)

This is a regression from bug 1077515. I bisected the regression to this pushlog:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a3665a95a357&tochange=203d3b5da245

CMD+Up and CMD+Down should scroll the web page to the top and bottom, respectively. After bug 1077515, they do nothing, but Option+Up and Option+Down now scroll to the top and bottom of the page instead.

[Tracking Requested - why for this release]:

This is serious regression for keyboard shortcuts.
Assignee: nobody → athena
Status: NEW → ASSIGNED
Attached patch cmd-up-down-bug-1105101.patch (obsolete) — Splinter Review
cmd_moveUp/Down3 doesn't exist, so pointing this keyboard shortcut to use cmd_moveUp/Down2. 

Do we want to keep the (new?) alt+up/down behavior?
Attachment #8528821 - Flags: review?(roc)
Keywords: dogfood
Comment on attachment 8528821 [details] [diff] [review]
cmd-up-down-bug-1105101.patch

Review of attachment 8528821 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/xbl/builtin/mac/platformHTMLBindings.xml
@@ +47,2 @@
>        <handler event="keypress" keycode="VK_LEFT" modifiers="accel" command="cmd_moveLeft3"/>
>        <handler event="keypress" keycode="VK_RIGHT" modifiers="accel" command="cmd_moveRight3"/>

While you're here, there's also a problem with these entries: cmd_move{Left,Right}3 don't exist either.

AFAICT, there's no existing behavior that we're losing because of this, so we could just remove these handlers. They were added while working on the patches in bug 1077515 for vertical/horizontal-mode symmetry, but I'm not sure we really want them; in any case, we can reconsider that later if necessary. For now, they're clearly erroneous.
Attached patch cmd-up-down-bug-1105101.patch (obsolete) — Splinter Review
Makes sense, removing those two additional entries.
Attachment #8528821 - Attachment is obsolete: true
Attachment #8528821 - Flags: review?(roc)
Attachment #8529147 - Flags: review?(roc)
(In reply to Athena from comment #1)
> Do we want to keep the (new?) alt+up/down behavior?

Not for OS X, at least. The new alt+up/down behavior, jumping to the top or bottom of the page, is not the standard behavior on OS X. These keyboard shortcuts should scroll the page up or down one screen at a time, like the pageup/pagedown keys.
(In reply to Chris Peterson (:cpeterson) from comment #4)
> (In reply to Athena from comment #1)
> > Do we want to keep the (new?) alt+up/down behavior?
> 
> Not for OS X, at least. The new alt+up/down behavior, jumping to the top or
> bottom of the page, is not the standard behavior on OS X.

Right, that was an inadvertent change and it's probably better to revert it.

> These keyboard
> shortcuts should scroll the page up or down one screen at a time, like the
> pageup/pagedown keys.

Note that they *don't* do this in current Firefox (33); alt-up/down have no effect.
(In reply to Jonathan Kew (:jfkthame) from comment #5)
> > These keyboard
> > shortcuts should scroll the page up or down one screen at a time, like the
> > pageup/pagedown keys.
> 
> Note that they *don't* do this in current Firefox (33); alt-up/down have no
> effect.

oh! I never noticed that. I just use the spacebar and shift+spacebar to scroll. <:)
Okay so we've ended up with this:

1) Remove cmd+left/right mapping - not mapped to anything right now; also interferes with history navigation as per 1105321.

2) Remove alt+up/down mapping - reverting to current (release) Firefox behavior of doing nothing

3) Remapped cmd+up/down to go to top and bottom of page
Attachment #8529147 - Attachment is obsolete: true
Attachment #8529147 - Flags: review?(roc)
Attachment #8529339 - Flags: review?(roc)
Keywords: checkin-needed
Hi, could you (or maybe roc) provide a try link, thanks!
Flags: needinfo?(roc)
Flags: needinfo?(athena)
Keywords: checkin-needed
Flags: needinfo?(athena) → needinfo?
Flags: needinfo?
Roc's try push appears to be stuck in the Thanksgiving logjam, so I pushed another one: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=953532d5ec49. And that looks green enough that I have landed the patch on inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/a1b62d172b55
Target Milestone: --- → mozilla37
https://hg.mozilla.org/mozilla-central/rev/a1b62d172b55
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Athena, could you fill the uplift request for aurora? Thanks
Flags: needinfo?(athena)
Comment on attachment 8529339 [details] [diff] [review]
cmd-up-down-bug-1105101.patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1077515
[User impact if declined]: Breaking expected conventions of keyboard shortcut behavior on a Mac (user muscle memory)

[Describe test coverage new/current, TBPL]: Manual testing on a Mac.

[Risks and why]: Should be low risk; the patch (1) remaps from a nonexistent function to an existing one (2) removes extra key mappings (appropriate for other OS conventions but not for a Mac)

[String/UUID change made/needed]: none
Attachment #8529339 - Flags: approval-mozilla-aurora?
Comment on attachment 8529339 [details] [diff] [review]
cmd-up-down-bug-1105101.patch

Thanks
Attachment #8529339 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(athena)
QA Whiteboard: [good first verify]
Component: Keyboard: Navigation → User events and focus handling
You need to log in before you can comment on or make changes to this bug.