Closed Bug 562849 Opened 13 years ago Closed 13 years ago

Reverse meanings of N900 volume buttons in portrait mode


(Firefox for Android Graveyard :: General, defect)

Not set


(Not tracked)



(Reporter: mbrubeck, Assigned: mbrubeck)



(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
On the N900, the "volume down" button in landscape mode becomes the "volume up" button in portrait mode (since it is now on the top).

This patch switches the volume rocker zoom controls depending on the device orientation.
Comment on attachment 442590 [details] [diff] [review]

>diff -r ce48d028c5ab chrome/content/browser-ui.js

>+  isPortrait: function isPortrait() {
>+    return (window.innerWidth < 500);
>+  },

We have an actual flag for this on the N900:

You can query it from JS like this:

Just check for "screen-orientation", instead of device. I wonder if Android will support the system-info "screen-orientation" though.

>+      case "cmd_volumeLeft":
>+      case "cmd_volumeRight":

Why add cmd_volumeLeft and Right? I assume you want to keep cmd_zoomin & out fixed, regardless of orientation?

>diff -r ce48d028c5ab chrome/content/browser.xul

> #ifndef WINCE
>     <key id="key_quit" key="q" modifiers="accel" command="cmd_quit"/>
>     <key id="key_fullscreen" keycode="VK_F6" command="cmd_fullscreen"/>
>     <key id="key_zoomin2" keycode="VK_F7" command="cmd_zoomin"/>
>     <key id="key_zoomout2" keycode="VK_F8" command="cmd_zoomout"/>
> #else
>     <key id="key_quit" keycode="VK_F4" command="cmd_quit"/>
>-    <key id="key_zoomin2" keycode="VK_F1" command="cmd_zoomin"/>
>-    <key id="key_zoomout2" keycode="VK_F2" command="cmd_zoomout"/>
>+    <key id="key_zoomin2" keycode="VK_F1" command="cmd_volumeRight"/>
>+    <key id="key_zoomout2" keycode="VK_F2" command="cmd_volumeLeft"/>
> #endif

Your in the wrong side of the #ifndef. You want to change the top block (F7 and F8)
Attachment #442590 - Flags: review-
Until we know how Android reacts, including whether we want to use the volume keys for zooming on Android, I guess using the window.innerWidth for a "portrait" check is OK.
Attached patch patch v2Splinter Review
Updated patch, tested on N900 this time - there was a reason I hadn't asked for a review yet!  :)

This patch still uses window.innerWidth as a proxy for orientation.  If we make this Maemo-only then I will switch to the Maemo-specific call.

> Why add cmd_volumeLeft and Right? I assume you want
> to keep cmd_zoomin & out fixed, regardless of orientation?

Yeah, exactly.  I didn't like the idea of a "zoomin" command that sometimes zooms out - especially if we later add other ways to activate the command (like menu items or buttons).
Attachment #442590 - Attachment is obsolete: true
Attachment #442778 - Flags: review?(mark.finkle)
Comment on attachment 442778 [details] [diff] [review]
patch v2

nit: I think we should move "isPortrait" to Util.isPortrait

I can do that on checkin.
Attachment #442778 - Flags: review?(mark.finkle) → review+
Yeah - we should do this.

On Android, at least with multitouch support, we shouldn't do volume key
pushed m-b:

pushed m-1.1:
Closed: 13 years ago
Resolution: --- → FIXED
verified FIXED on builds:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv: Gecko/20100503 Namoroka/3.6.5pre Fennec/1.1b2pre


Mozilla/5.0 (X11; U; Linux armv71; Nokia N900; en-US; rv:1.9.3a5pre) Gecko/20100503 Namoroka/3.7a5pre Fennec/1.1b2pre
Flags: in-litmus?
litmus testcase updated
Flags: in-litmus? → in-litmus+
Component: Linux/Maemo → General
QA Contact: maemo-linux → general
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.