The default bug view has changed. See this FAQ.

Keyboard control for zoom in tilt (page inspector 3D)

VERIFIED FIXED in Firefox 11

Status

()

Firefox
Developer Tools
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: Kevin Dangoor, Assigned: vporof)

Tracking

({verified-beta})

unspecified
Firefox 12
x86
Mac OS X
verified-beta
Points:
---

Firefox Tracking Flags

(firefox11+ verified)

Details

(Whiteboard: [tilt][fixed-in-fx-team][qa!])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Victor tells me that there's no way right now to zoom using just the keyboard in 3D mode in the page inspector. It would be good to have one. The = and - keys seem like good options (for zoom in/out, respectively). Supporting shift+those keys would be good as well, so that the user could push + for zoom in.
(Assignee)

Updated

5 years ago
Whiteboard: [tilt]
(Assignee)

Comment 1

5 years ago
Created attachment 581938 [details] [diff] [review]
v1
Attachment #581938 - Flags: review?(rcampbell)
Comment on attachment 581938 [details] [diff] [review]
v1

+    if (str === "+" || str === "=") {
+      this.arcball.mouseScroll(-ARCBALL_TRANSLATION_STEP);
+    } else if (str === "-" || str === "_") {
+      this.arcball.mouseScroll(ARCBALL_TRANSLATION_STEP);
+    }

not sure all non-US keyboards have this arrangement.

see http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/browser.dtd#471

Should probably mimic those keys.

Also, would be good to have a zoom reset key on Cmd/ctrl-0.

We should probably verify this with a test as well.
Attachment #581938 - Flags: review?(rcampbell) → review-
(Assignee)

Comment 3

5 years ago
Created attachment 582506 [details] [diff] [review]
v2

Don't ask me how I write these tests.
Attachment #581938 - Attachment is obsolete: true
Attachment #582506 - Flags: review?(rcampbell)
(Assignee)

Comment 4

5 years ago
Created attachment 582518 [details] [diff] [review]
v3

It would also probably best if zooming via keyboard and mouse would be unified in the arcball implementation. Added fix in here because a new bug would seem too much for this.
Attachment #582518 - Flags: review?(rcampbell)
Comment on attachment 582518 [details] [diff] [review]
v3

   /**
    * Object retaining the current pressed key codes.
@@ -1114,6 +1109,7 @@
   this._lastTrans = vec3.create();
   this._deltaTrans = vec3.create();
   this._currentTrans = vec3.create(aInitialTrans);
+  this._zoomAmmount = 0;
 
zoomAmount (typo!)

 
-    let scrollValue = this._scrollValue;
+    let zoomAmmount = this._zoomAmmount;
     let keyCode = this._keyCode;

+    if (keyCode[Ci.nsIDOMKeyEvent.DOM_VK_I] ||
+        keyCode[Ci.nsIDOMKeyEvent.DOM_VK_ADD] ||
+        keyCode[Ci.nsIDOMKeyEvent.DOM_VK_EQUALS]) {

these are going to be impossible to localize. Kind of the reason I wanted to sniff the keycodes off of the document's page zoom keys. Either that or put them in a properties file.

Certainly for I, O and R those keys may not make sense in other locales. +/- and = may be ok, but I've never really looked to see how they get localized elsewhere.

It also looks like we could replace all the previous if(key) checks with a switch statement.

+   * Function handling the arcball zoom ammount.

pesky typo again.

So, typos fixed, not fully sure what to do about the l10n issue. Might be ok for a follow-up.

Axel, do you have any recommendations for zoom key controls? Is sniffing the DOM for the current zoom keys an acceptable solution or should we bundle these into a properties file? Thanks.
Attachment #582518 - Flags: review?(l10n)
(Assignee)

Comment 6

5 years ago
(In reply to Rob Campbell [:rc] (robcee) from comment #5)
> 
> these are going to be impossible to localize. Kind of the reason I wanted to
> sniff the keycodes off of the document's page zoom keys. Either that or put
> them in a properties file.
> 
> Certainly for I, O and R those keys may not make sense in other locales. +/-
> and = may be ok, but I've never really looked to see how they get localized
> elsewhere.
> 

Then maybe we should localize ALL the keys? (even for up/down/left/right and rotations..)

> 
> So, typos fixed, not fully sure what to do about the l10n issue. Might be ok
> for a follow-up.
> 
> Axel, do you have any recommendations for zoom key controls? Is sniffing the
> DOM for the current zoom keys an acceptable solution or should we bundle
> these into a properties file? Thanks.

Afaik, sniffing would be bad because it returns a string ("+" for zoom, etc.). It's not safe to get the correct keycode from that. (For example, on my keyboard "-" is interpreted as the same keycode for DOM_VK_INSERT, and doing fromCharCode returns "M" for the keyCode I get when I press "-").
hm, yeah. OK.

Maybe we can file a bug to make the other keys localizeable. Want to file a bug for that?

In the meantime we can get this in for this release (with the typo fixed).
Status: NEW → ASSIGNED
(Assignee)

Comment 8

5 years ago
Created attachment 582816 [details] [diff] [review]
v4

Fixed typo. Followup bug for localization would be perfect.
Attachment #582816 - Flags: review?(rcampbell)
Comment on attachment 582816 [details] [diff] [review]
v4

ok!
Attachment #582816 - Flags: review?(rcampbell) → review+
https://hg.mozilla.org/integration/fx-team/rev/de1cf77a8d6b
tracking-firefox11: --- → ?
Whiteboard: [tilt] → [tilt][fixed-in-fx-team]
Comment on attachment 582816 [details] [diff] [review]
v4

This would be nice to get into aurora. It adds page zoom controls on +, -, I, O keys. It even has a test!
Attachment #582816 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 12

5 years ago
For the record, there's the followup bug 712026 for localization.
https://hg.mozilla.org/mozilla-central/rev/de1cf77a8d6b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12

Comment 14

5 years ago
Comment on attachment 582816 [details] [diff] [review]
v4

[triage comment]
Approved for aurora. While this probably wouldn't meet our criteria at the end of a cycle ("nice to have", feature works fine without, etc). We are early in the cycle and the risk looks low to take this for what will likely be a banner feature.
Attachment #582816 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

5 years ago
Attachment #582518 - Flags: review?(l10n)
https://hg.mozilla.org/releases/mozilla-aurora/rev/37d9a6bb1156
status-firefox11: --- → fixed

Updated

5 years ago
tracking-firefox11: ? → +
Comment on attachment 582518 [details] [diff] [review]
v3

clearing old requests.
Attachment #582518 - Flags: review?(rcampbell)
Attachment #582506 - Flags: review?(rcampbell)
Whiteboard: [tilt][fixed-in-fx-team] → [tilt][fixed-in-fx-team][qa+]
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0a2) Gecko/20120206 Firefox/12.0a2

Verified the fix on the latest beta build (11.0 beta 1). "-"/"=", "I"/"O", "-"/"+" keys support zooming in and out in 3D mode in the page inspector.
Status: RESOLVED → VERIFIED
status-firefox11: fixed → verified
Keywords: verified-beta
Whiteboard: [tilt][fixed-in-fx-team][qa+] → [tilt][fixed-in-fx-team][qa!]
You need to log in before you can comment on or make changes to this bug.