Last Comment Bug 456520 - Firefox should support Multi-Touch Gestures on Mac OS X
: Firefox should support Multi-Touch Gestures on Mac OS X
Status: VERIFIED FIXED
: dev-doc-complete, user-doc-complete
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All Mac OS X
: -- enhancement with 1 vote (vote)
: Firefox 3.1b1
Assigned To: Tom Dyas
:
Mentors:
Depends on: 412486 483499 491925
Blocks: FF2SM 478606 440628 461376 465265 469768 478607 478666 478939
  Show dependency treegraph
 
Reported: 2008-09-22 20:22 PDT by Tom Dyas
Modified: 2009-10-29 00:25 PDT (History)
20 users (show)
bugs: in‑testsuite+
mozillamarcia.knous: in‑litmus+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Firefox-Specific Gestures Support v4 (14.23 KB, patch)
2008-09-22 20:26 PDT, Tom Dyas
no flags Details | Diff | Splinter Review
Firefox-Specific Gestures Support v5 (14.21 KB, patch)
2008-09-22 21:06 PDT, Tom Dyas
edilee: review-
Details | Diff | Splinter Review
ed.patch v1 (9.04 KB, patch)
2008-10-21 11:13 PDT, Ed Lee :Mardak
no flags Details | Diff | Splinter Review
ed.patch v1.1 (13.93 KB, patch)
2008-10-21 11:26 PDT, Ed Lee :Mardak
sdwilsh: review+
Details | Diff | Splinter Review
v5.1.1 (15.20 KB, patch)
2008-10-21 14:48 PDT, Ed Lee :Mardak
mbeltzner: ui‑review+
Details | Diff | Splinter Review

Description Tom Dyas 2008-09-22 20:22:42 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080922213520 Minefield/3.1b1pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080922213520 Minefield/3.1b1pre

See bug 412486 for patch implementing widget-level support for the Mac OS X multi-touch gestures.  That patch is in the review phase.  I am opening this bug so that my Firefox-specific patch can be reviewed in parallel.

Basically, with the base patch in bug 412486 and the attached patch, Firefox supports the same swipe and magnify gestures as Safari.  I also added a "rotate through tabs" response for the rotate gesture.

Reproducible: Always
Comment 1 Tom Dyas 2008-09-22 20:26:39 PDT
Created attachment 339916 [details] [diff] [review]
Firefox-Specific Gestures Support v4

Multi-Touch gesture support for Firefox.  Depends on attachment 339910 [details] [diff] [review] to bug 412486.
Comment 2 Tom Dyas 2008-09-22 21:06:32 PDT
Created attachment 339923 [details] [diff] [review]
Firefox-Specific Gestures Support v5

Hook |window| instead of |gBrowser|.
Comment 3 Tom Dyas 2008-10-02 08:04:57 PDT
The base patch passed super-review.

Please note that this patch supports 3 gestures not supported by Safari: (1) three-fingered swipe "up" goes to top of page; (2) three-fingered swipe "down" does to the bottom of page; and (3) rotation moves through tabs.  Do these non-standard gestures need to be approved by someone?
Comment 4 S Woodside 2008-10-04 19:19:05 PDT
Rotate: moving through tabs seems like a good idea. I can't see any direct use for rotation in a web browser.

Three-finger swipe: top/bottom page seems somewhat useful, another possibility is for a sort of hyper-scroll, where it scrolls much faster than a regular two-finger (pixel) scroll. That way, you could quickly scroll to the top of even a long page, but if you want to go some way up, you could also do that. Do you get incremental amounts in a three-finger swipe, or just a single swipe event?
Comment 5 Tom Dyas 2008-10-04 19:34:52 PDT
(In reply to comment #4)
>
> Three-finger swipe: top/bottom page seems somewhat useful, another possibility
> is for a sort of hyper-scroll, where it scrolls much faster than a regular
> two-finger (pixel) scroll. That way, you could quickly scroll to the top of
> even a long page, but if you want to go some way up, you could also do that. Do
> you get incremental amounts in a three-finger swipe, or just a single swipe
> event?

Mac OS X only reports incremental amounts for the pinch and rotate gestures. Only one event per swipe is sent.  (Swipes are also not bracketed by the "begin gesture" and "end gesture" events like the other two.)
Comment 6 Tom Dyas 2008-10-04 19:45:08 PDT
Who are the appropriate reviewer and super-reviewer for patches like this?
Comment 7 Ed Lee :Mardak 2008-10-21 11:12:10 PDT
Comment on attachment 339923 [details] [diff] [review]
Firefox-Specific Gestures Support v5

I can't give an official review for this area, but here's some feedback if you want to use any of it. :) I've attached a patch on top of yours that implements some of what I've commented on.

>diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js
>+#   Thomas K. Dyas <tdyas@zecador.org> (simple gestures support)
I don't think qualifying what you added is necessary. Otherwise you'll have to keep changing it when you contribute some more ;)

>+var gGestureSupport = {
>+  init : function() {
>+    window.addEventListener("MozSwipeGesture", this.onSwipeGesture, true);
>+    window.addEventListener("MozMagnifyGestureStart", this.onMagnifyGestureStart, true);
>+    window.addEventListener("MozMagnifyGestureUpdate", this.onMagnifyGestureUpdate, true);
>+    window.addEventListener("MozMagnifyGesture", this.stopEvent, true);
>+    window.addEventListener("MozRotateGestureStart", this.onRotateGesture, true);
>+    window.addEventListener("MozRotateGestureUpdate", this.onRotateGesture, true);
>+    window.addEventListener("MozRotateGesture", this.stopEvent, true);
>+  },
>+
>+  uninit : function() {
>+    window.removeEventListener("MozSwipeGesture", this.onSwipeGesture, true);
>+    window.removeEventListener("MozMagnifyGestureStart", this.onMagnifyGestureStart, true);
>+    window.removeEventListener("MozMagnifyGestureUpdate", this.onMagnifyGestureUpdate, true);
>+    window.removeEventListener("MozMagnifyGesture", this.stopEvent, true);
>+    window.removeEventListener("MozRotateGestureStart", this.onRotateGesture, true);
>+    window.removeEventListener("MozRotateGestureUpdate", this.onRotateGesture, true);
>+    window.removeEventListener("MozRotateGesture", this.stopEvent, true);
>+  },
This could be simplified to have all the events handled by single event listener that first calls stopPropagation() then dispatches to the desired event handler.

>+  onSwipeGesture : function (evt) {
>+    if (evt.direction == SimpleGestureEvent.DIRECTION_LEFT) {
>+      var fakeEvent = { shiftKey: evt.shiftKey,
>+                        ctrlKey: evt.ctrlKey,
>+                        metaKey: evt.metaKey,
>+                        altKey: evt.altKey,
>+                        button: 0 };
>+      BrowserBack(fakeEvent);
>+    } else if (evt.direction == SimpleGestureEvent.DIRECTION_RIGHT) {
>+      var fakeEvent = { shiftKey: evt.shiftKey,
>+                        ctrlKey: evt.ctrlKey,
>+                        metaKey: evt.metaKey,
>+                        altKey: evt.altKey,
>+                        button: 0 };
Probably cleaner to just move the declaration outside of the if blocks and then you can just drop the {}s off.

>+  _initiallyZoomedOut : false,
>+  _isZoomed : false,
>+
>+  onMagnifyGestureStart : function (evt) {
>+    evt.stopPropagation();
>+
>+    if (evt.delta > 0.0) {
>+      FullZoom.enlarge();
>+      this._initiallyZoomedOut = true;
>+    } else {
>+      FullZoom.reduce();
>+      this._initiallyZoomedOut = false;
>+    }
>+
>+    this._isZoomed = true;
>+  },
>+
>+  onMagnifyGestureUpdate : function (evt) {
>+    evt.stopPropagation();
>+
>+    if (this._initiallyZoomedOut) {
>+      if (evt.delta > 0.0 && ! this._isZoomed) {
>+        FullZoom.enlarge();
>+        this._isZoomed = true;
>+      } else if (evt.delta < 0.0 && this._isZoomed) {
>+        FullZoom.reduce();
>+        this._isZoomed = false;
>+      }
>+    } else {
>+      if (evt.delta < 0.0 && ! this._isZoomed) {
>+        FullZoom.reduce();
>+        this._isZoomed = true;
>+      } else if (evt.delta > 0.0 && this._isZoomed) {
>+        FullZoom.enlarge();
>+        this._isZoomed = false;
>+      }
>+    }
>+  },
Why such a complex behavior for zoom depending on if it's the start vs an update? It seems like this would run into problems if the user changes the zoom level with the keyboard like cmd-0 to reset zoom.

>+  _currentAngle : 0.0,
>+
>+  computeThresholdMultiple : function (angle) {
>+    var multiple = angle / 22.5;
>+
>+    if (multiple > 0.0)
>+      multiple = Math.floor(multiple);
>+    else
>+      multiple = Math.ceil(multiple);
>+
>+    return multiple;
>+  },
>+
>+  onRotateGesture : function (evt) {
>+    evt.stopPropagation();
>+
>+    // Initialize the current angle at the start of the gesture.
>+    if (evt.type == "MozRotateGestureStart") {
>+      this._currentAngle = 0.0;
>+    }
>+
>+    // Update the current angle and save away the old angle.
>+    var oldAngle = this._currentAngle;
>+    this._currentAngle = this._currentAngle + evt.delta;
>+
>+    // Compute the old and new multiples of the threshold angle.
>+    var oldThresholdMultiple = gGestureSupport.computeThresholdMultiple(oldAngle);
>+    var newThresholdMultiple = gGestureSupport.computeThresholdMultiple(this._currentAngle);
The computeThresholdMultiple is basically trying to figure out how far the user has rotated in terms of coarser-grained ticks. Instead of calculating ticks then subtracting, you could first subtract then compute the ticks once.

>+    // Based on the direction of the difference, advance the selected
>+    // tab left or right.
>+    var diff = newThresholdMultiple - oldThresholdMultiple;
>+    while (diff > 0) {
>+      gBrowser.mTabContainer.advanceSelectedTab(1, true);
>+      diff--;
>+    }
>+    while (diff < 0) {
>+      gBrowser.mTabContainer.advanceSelectedTab(-1, true);
>+      diff++;
>+    }
Do you expect the user to switch through multiple tabs in one rotation? Would it be good to handle switching through multiple tabs or just have it switch at most one tab at a time?

>diff --git a/browser/base/content/test/Makefile.in b/browser/base/content/test/Makefile.in
>+		 browser_bug412486.js \
nit: spaces instead of tabs

>diff --git a/browser/base/content/test/browser_bug412486.js b/browser/base/content
>+function test_EnsureConstantsAreDisjoint()
>+  var up = SimpleGestureEvent.DIRECTION_UP;
>+  var down = SimpleGestureEvent.DIRECTION_DOWN;
>+  ok((up & down) == 0, "DIRECTION_UP and DIRECTION_DOWN are not bitwise disjoint");
>+  is(up | down, up + down, "DIRECTION_UP and DIRECTION_DOWN are not bitwise disjoint #2");
You can ensure the same thing with a single test :)

ok(up ^ down, "up/down should be bitwise disjoint");
Comment 8 Ed Lee :Mardak 2008-10-21 11:13:21 PDT
Created attachment 344132 [details] [diff] [review]
ed.patch v1

Here's my suggestions from the previous comment as well as some simplifications and added comments.
Comment 9 Ed Lee :Mardak 2008-10-21 11:26:17 PDT
Created attachment 344135 [details] [diff] [review]
ed.patch v1.1

Whoops. Missed a couple changes in last second refactoring ;) Made it a plain patch instead of on top of the previous patch.
Comment 10 Ed Lee :Mardak 2008-10-21 11:32:03 PDT
(In reply to comment #3)
> Please note that this patch supports 3 gestures not supported by Safari: (1)
> three-fingered swipe "up" goes to top of page; (2) three-fingered swipe "down"
> does to the bottom of page; and (3) rotation moves through tabs.  Do these
> non-standard gestures need to be approved by someone?

Beltzner?

The rotate through tabs is really neat :D After just using it for about an hour through testing, plain trunk without motions = super boring :p
Comment 11 Dão Gottwald [:dao] 2008-10-21 11:59:06 PDT
Comment on attachment 344135 [details] [diff] [review]
ed.patch v1.1

>+  init: function(aInit) {

>+      addRemove("Moz" + event, this.handleEvent, true);

addRemove("Moz" + event, this, true);

>+  handleEvent: function(aEvent) {
>+    aEvent.stopPropagation();
>+
>+    switch (aEvent.type) {
>+      case "MozSwipeGesture":
>+        gGestureSupport.onSwipe(aEvent);

this.onSwipe(aEvent);

same below

>+++ b/browser/base/content/test/Makefile.in
>@@ -66,6 +66,7 @@
>                  page_style_sample.html \
>                  browser_ctrlTab.js \
>                  browser_selectTabAtIndex.js \
>+                 browser_bug412486.js \

browser_gestureSupport.js?
Comment 12 Shawn Wilsher :sdwilsh 2008-10-21 12:00:11 PDT
Comment on attachment 344135 [details] [diff] [review]
ed.patch v1.1

Can we not use anonymous functions in the JS object please?  It makes things like dtrace and the JS debugger way more useful.

>+// Simple gestures support
>+//
>+// As per bug #412486, web content must not be allowed to receive any
>+// simple gesture events.  Multi-touch gesture APIs are in their
>+// infancy and we do NOT want to be forced into supporting an API that
>+// will probably have to change in the future.  (The current Mac OS X
>+// API is undocumented and was reverse-engineered.)  Until support is
>+// implemented in the event dispatcher to keep these events as
>+// chrome-only, we must listen for the simple gesture events during
>+// the capturing phase and call stopPropagation on every event.
Can we file a bug on having the event not dispatch to content without this and reference that bug in this comment please?  Seems like something we should be doing.

>+let gGestureSupport = {
>+  /**
>+   * Add or remove mouse gesture event listeners
>+   *
>+   * @param aInit
>+   *        True to add/init listeners and false to remove/uninit
>+   */
>+  init: function(aInit) {
>+    let addRemove = aInit ? window.addEventListener : window.removeEventListener;
>+    for each (let event in ["SwipeGesture", "MagnifyGestureStart",
>+                "MagnifyGestureUpdate", "MagnifyGesture", "RotateGesture",
>+                "RotateGestureUpdate", "RotateGesture"])
Can we get the array pulled out into a const please?

>+  onMagnify: function(aEvent) {
>+    this._handleUpdate(aEvent, 100, function(aOffset) aOffset > 0 ?
>+      FullZoom.enlarge() : FullZoom.reduce());
>+  },
just checking - this will save the setting in site specific preferences still, right?

I didn't review the tests, but r=sdwilsh.  This should be cool.
Comment 13 Dão Gottwald [:dao] 2008-10-21 12:01:12 PDT
init(aInit) is also pretty weird...
Comment 14 Tom Dyas 2008-10-21 12:25:05 PDT
(In reply to comment #7)
> (From update of attachment 339923 [details] [diff] [review])
> I can't give an official review for this area, but here's some feedback if you
> want to use any of it. :) I've attached a patch on top of yours that implements
> some of what I've commented on.
>
> Why such a complex behavior for zoom depending on if it's the start vs an
> update? It seems like this would run into problems if the user changes the zoom
> level with the keyboard like cmd-0 to reset zoom.

The start and update events are linked to particular pinch gestures.  (The widget code generates a roll-up event (MozMagnifyGesture) at the end of each pinch.  However, it is not used here because it doesn't allow the user to see the text size change dynamically during the gesture.)

Basically if the user pinches out, the code zooms the text, "latches" that status, and any further pinch out does not increase the text size during that gesture.  But if, during the same gesture, the user pinches in, the text is unzoomed and that status "latches" until the user pinches out.  If you want to zoom out two levels, you have to do two pinch out gestures in a row (which entails removing your fingers from the trackpad in between).  This matches the Safari behavior.

The code may seem a bit convoluted because it also handles the case where the initial direction of the pinch gesture was a pinch in.

> The computeThresholdMultiple is basically trying to figure out how far the user
> has rotated in terms of coarser-grained ticks. Instead of calculating ticks
> then subtracting, you could first subtract then compute the ticks once.

My thinking was to separate the rotate gesture into 22.5 degree zones.  As the user moves from zone to zone, the tabs rotate.  The subtraction merely determines what direction the user rotated in.

> Do you expect the user to switch through multiple tabs in one rotation? Would
> it be good to handle switching through multiple tabs or just have it switch at
> most one tab at a time?

The user should be able to switch through multiple tabs during a single rotation.  We can play with the 22.5 degree threshold but making the user remove their fingers from the trackpad between each tab rotation is bound to annoy them.  (They would have to do this if we forced them to make multiple rotation gestures.)
Comment 15 Tom Dyas 2008-10-21 12:42:06 PDT
(In reply to comment #12)
> (From update of attachment 344135 [details] [diff] [review])
> >+// Simple gestures support
> >+//
> >+// As per bug #412486, web content must not be allowed to receive any
> >+// simple gesture events.  Multi-touch gesture APIs are in their
> >+// infancy and we do NOT want to be forced into supporting an API that
> >+// will probably have to change in the future.  (The current Mac OS X
> >+// API is undocumented and was reverse-engineered.)  Until support is
> >+// implemented in the event dispatcher to keep these events as
> >+// chrome-only, we must listen for the simple gesture events during
> >+// the capturing phase and call stopPropagation on every event.

> Can we file a bug on having the event not dispatch to content without this and
> reference that bug in this comment please?  Seems like something we should be
> doing.

I'll have to check whether a bug is already filed on adding this functionality.  I know it was discussed under bug 412486.
Comment 16 Tom Dyas 2008-10-21 12:50:51 PDT
Ed: Your patch checks whether the magnify gesture has reached a specific value before doing the zoom.  The units that Mac OS X uses for zoom gestures are unknown and should not be relied upon.  (I documented this in the base patch.  See dom/public/idl/events/nsIDOMSimpleGestureEvent.idl)  This is the reason why my zoom handling only relies on the sign of the delta and not its magnitude.  Safari's behavior is the same.
Comment 17 Ed Lee :Mardak 2008-10-21 14:48:05 PDT
Created attachment 344187 [details] [diff] [review]
v5.1.1

Try server builds are available for those using 2008 macbook pros, macbook airs, or late 2008 macbooks.
https://build.mozilla.org/tryserver-builds/2008-10-21_11:36-edward.lee@engineering.uiuc.edu-multi.touch/edward.lee@engineering.uiuc.edu-multi.touch-firefox-try-mac.dmg

(Who came up with this 5.1.1 versioning scheme?! :p)

(In reply to comment #11)
> >+  init: function(aInit) {
> >+      addRemove("Moz" + event, this.handleEvent, true);
> addRemove("Moz" + event, this, true);
> 
> >+        gGestureSupport.onSwipe(aEvent);
> this.onSwipe(aEvent);
Neato. Gets rid of the issue of |this| being ChromeWindow instead of gGestureSupport in handleEvent. :)

> >+                 browser_bug412486.js \
> browser_gestureSupport.js?
Renamed.

(In reply to comment #12)
> Can we not use anonymous functions in the JS object please?  It makes things
> like dtrace and the JS debugger way more useful.
Added GS_<func> for each function.

> >+    for each (let event in ["SwipeGesture", "MagnifyGestureStart",
> >+                "MagnifyGestureUpdate", "MagnifyGesture", "RotateGesture",
> >+                "RotateGestureUpdate", "RotateGesture"])
> Can we get the array pulled out into a const please?
Moved into a const within that function.

> >+  onMagnify: function(aEvent) {
> >+    this._handleUpdate(aEvent, 100, function(aOffset) aOffset > 0 ?
> >+      FullZoom.enlarge() : FullZoom.reduce());
> >+  },
> just checking - this will save the setting in site specific preferences still,
> right?
Yup. And if you switch to a tab of the same domain, things will switch to that zoom level.

> I didn't review the tests, but r=sdwilsh.  This should be cool.
I've updated the test to use the init(true/false) and saw that it passes. Also cleaned up some whitespace and boilerplate, etc.

(In reply to comment #13)
> init(aInit) is also pretty weird...
Made it.. init: function GS_init(aAddListener) {
Comment 18 Ed Lee :Mardak 2008-10-21 14:49:43 PDT
Comment on attachment 344187 [details] [diff] [review]
v5.1.1

Beltzner asked on IRC about why not swipe for tab switching.

Personally, I was about to file a bug today about 3finger right/left to do switch tab (and found this bug instead). But after playing around with rotate, it seems to be a better fit. One big reason is that you can continuously rotate in a single motion to switch multiple tabs.
Comment 19 Ed Lee :Mardak 2008-10-21 15:00:23 PDT
(In reply to comment #14)
> > The computeThresholdMultiple is basically trying to figure out how far the user
> > has rotated in terms of coarser-grained ticks. Instead of calculating ticks
> > then subtracting, you could first subtract then compute the ticks once.
> My thinking was to separate the rotate gesture into 22.5 degree zones.  As the
> user moves from zone to zone, the tabs rotate.  The subtraction merely
> determines what direction the user rotated in.
Yeah, I was just suggesting that instead of doing |diff = angleToZone(old) - angleToZone(new)|, you can get something similar with |diff = angleToZone(old - new)|.

> > Do you expect the user to switch through multiple tabs in one rotation? Would
> > it be good to handle switching through multiple tabs or just have it switch at
> > most one tab at a time?
> The user should be able to switch through multiple tabs during a single
> rotation.  We can play with the 22.5 degree threshold but making the user
> remove their fingers from the trackpad between each tab rotation is bound to
> annoy them.  (They would have to do this if we forced them to make multiple
> rotation gestures.)
To clarify, I meant switching multiple tabs in a single gesture event update. (FYI, the v5.1.1 patch does allow switching multiple tabs in a single gesture with multiple updates because it resets the offset when it triggers an action.)

(In reply to comment #16)
> Ed: Your patch checks whether the magnify gesture has reached a specific value
> before doing the zoom.  The units that Mac OS X uses for zoom gestures are
> unknown and should not be relied upon.  (I documented this in the base patch. 
> See dom/public/idl/events/nsIDOMSimpleGestureEvent.idl)  This is the reason why
> my zoom handling only relies on the sign of the delta and not its magnitude. 
> Safari's behavior is the same.
Oh.

Hrmm.. Would it be totally bad if we used the magnitude? The rotation and zooming code do very similar things -- do an action multiple times if the user keeps going the same way, and if the user goes backwards in the same motion, handle it accordingly.
Comment 20 Tom Dyas 2008-10-21 16:38:56 PDT
(In reply to comment #18)
> (From update of attachment 344187 [details] [diff] [review])
> Beltzner asked on IRC about why not swipe for tab switching.
> 
> Personally, I was about to file a bug today about 3finger right/left to do
> switch tab (and found this bug instead). But after playing around with rotate,
> it seems to be a better fit. One big reason is that you can continuously rotate
> in a single motion to switch multiple tabs.

Safari uses the left/right swipes for backward and forward.  (The up/down swipe behavior is my addition.)  I would say that Firefox/Mac default shouldn't deviate from the Safari behavior.  Of course, it could always be a config option.

(In reply to comment #19)
> (In reply to comment #14)
> > > The computeThresholdMultiple is basically trying to figure out how far the user
> > > has rotated in terms of coarser-grained ticks. Instead of calculating ticks
> > > then subtracting, you could first subtract then compute the ticks once.
> > My thinking was to separate the rotate gesture into 22.5 degree zones.  As the
> > user moves from zone to zone, the tabs rotate.  The subtraction merely
> > determines what direction the user rotated in.
> Yeah, I was just suggesting that instead of doing |diff = angleToZone(old) -
> angleToZone(new)|, you can get something similar with |diff = angleToZone(old -
> new)|.

You're right.  Both have a common denominator (22.5) so it can be pulled out.  Ah algebra!

> > > Do you expect the user to switch through multiple tabs in one rotation? Would
> > > it be good to handle switching through multiple tabs or just have it switch at
> > > most one tab at a time?
> > The user should be able to switch through multiple tabs during a single
> > rotation.  We can play with the 22.5 degree threshold but making the user
> > remove their fingers from the trackpad between each tab rotation is bound to
> > annoy them.  (They would have to do this if we forced them to make multiple
> > rotation gestures.)
> To clarify, I meant switching multiple tabs in a single gesture event update.
> (FYI, the v5.1.1 patch does allow switching multiple tabs in a single gesture
> with multiple updates because it resets the offset when it triggers an action.)

Understood.  I believe that visual feedback to the user during the gesture in seeing the tabs switch is essential.  (Speaking as the only user other than you of the rotation gesture!)

> (In reply to comment #16)
> > Ed: Your patch checks whether the magnify gesture has reached a specific value
> > before doing the zoom.  The units that Mac OS X uses for zoom gestures are
> > unknown and should not be relied upon.  (I documented this in the base patch. 
> > See dom/public/idl/events/nsIDOMSimpleGestureEvent.idl)  This is the reason why
> > my zoom handling only relies on the sign of the delta and not its magnitude. 
> > Safari's behavior is the same.
> Oh.
> 
> Hrmm.. Would it be totally bad if we used the magnitude? The rotation and
> zooming code do very similar things -- do an action multiple times if the user
> keeps going the same way, and if the user goes backwards in the same motion,
> handle it accordingly.

Yes, I believe so.  As Apple has not documented the multi-touch gestures interface yet, there is no way to know how the values correlate to the size of actual movements of the user's fingers on the trackpad.  I have only been testing on my laptop (a Feb. refresh Macbook Pro).  The Macbook Air could conceivably assign different values to similar size finger movements.

I did some experimentation early on to determine that the units for rotation are "degrees."  I couldn't reach a similar conclusion for the pinch gesture.

My original "latching" code actually only does the zoom in/out once per the user moving in the same direction.  It matches the Safari behavior precisely.  Different concept than just doing an action repetitively.
Comment 21 Mike Beltzner [:beltzner, not reading bugmail] 2008-10-23 01:10:58 PDT
Comment on attachment 344187 [details] [diff] [review]
v5.1.1

ui-r=beltzner

I haven't found time to try this on a macbook with gestures (will do that on Friday) so I we might end up doing some tweaking in the future in terms of sensitivity of the rotation, exact choice of gestures, etc. However, I do want to get things into the tree quicker, so let's do.

The one nit I have is that I'm not totally sure of the value of 3fsUP (scroll-to-top) 3fsDOWN (scroll-to-bottom). Was wondering if those might better map to "new tab" and "close tab" or something that's more commonly used. Let's go with what you have for now, but think about it, ok?
Comment 22 Andrew Ferguson 2008-10-23 04:34:17 PDT
> The one nit I have is that I'm not totally sure of the value of 3fsUP
> (scroll-to-top) 3fsDOWN (scroll-to-bottom). Was wondering if those might better
> map to "new tab" and "close tab" or something that's more commonly used. Let's
> go with what you have for now, but think about it, ok?

From a user's perspective, I really like that 3fsUP and 3fsDOWN are scroll-to-top and scroll-to-bottom, because if I'm scrolling through a webpage, my fingers are already on the trackpad doing two-finger-scrolling.

Although I commonly use "new tab", I would not want it to be a mouse gesture since my fingers need to be at the keyboard anyway to type the URL into the new tab. Hitting command-t in that case flows naturally.

Perhaps all of the mouse gestures can be preferences? I do like the idea of the "hyper-scroll" proposed in comment #4.

Thanks for preparing these multi-touch patches!
Comment 23 Ed Lee :Mardak 2008-10-23 06:29:27 PDT
I also find 3-finger scrolling more useful than I thought I would. Jumping to the bottom is useful for long/slow loading pages such as comments or forum posts, and you're waiting for the rest of the page. Additionally, the bottom usually has the "continue" links or reply textarea whereas the top has useful navigation links.

About the "new tab" from mouse. It might not seem too unlikely that the user wouldn't need to type anything if we get the more useful new tab page.

But customizable would be nice -- we still need to pick a good set of default actions though.
Comment 24 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2008-10-23 16:46:08 PDT
author	tdyas@zecador.org
	Thu Oct 23 23:15:22 2008 +0300 (at Thu Oct 23 23:15:22 2008 +0300)
changeset 20794	b7bd85f1dd75
parent 20793	f39ecb5b05b7
child 20796	282f5674d429
Bug 456520 - Implementation of Multi-Touch Gestures on Mac OS X, p=tdyas@zecador.org+edilee@gmail.com, r=sdwilsh@forerunnerdesigns.com, ui-r=beltzner@mozilla.com
Comment 25 Marcia Knous [:marcia - use ni] 2009-02-04 12:50:14 PST
Using  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090203 Shiretoko/3.1b3pre. I verified the that three gestures mentioned in Comment 3 work on a Macbook Air.

Regarding http://www.apple.com/macbookair/features.html, is it expected that all the gestures on that page will work for Firefox?
Comment 26 Henrik Skupin (:whimboo) 2009-02-14 20:10:31 PST
Verified on the new MacBookPro with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090214 Shiretoko/3.1b3pre Ubiquity/0.1.5 ID:20090214020701

(In reply to comment #25)
> Regarding http://www.apple.com/macbookair/features.html, is it expected that
> all the gestures on that page will work for Firefox?

They should, yes. All of them are working fine for me in the browser window.
Comment 27 Henrik Skupin (:whimboo) 2009-02-15 10:04:09 PST
Smaug, how good is the coverage of the automated tests? Do we need litmus tests for this feature?
Comment 28 Marcia Knous [:marcia - use ni] 2009-02-26 10:51:56 PST
I went ahead and added a Litmus test to the Mac specific test suites, https://litmus.mozilla.org/show_test.cgi?id=7543.
Comment 29 Alex Faaborg [:faaborg] (Firefox UX) 2009-03-02 16:02:27 PST
Some feedback I received in email awhile ago, no idea if it is still an issue (don't have a new macbook to test get a feel for the changes):

>Hi Alex -- not sure who to send this to, so trying you ---
>
>I've just been watching my wife use FF 3.1b2, and she's accidentally pinch-
>zooming on her new macbook over and over again, making gmail almost unusable.  
>it's really getting in the way, and it's totally mysterious if you don't know 
>what's going on.

>Clearly part of this is an overally OS X problem, but it also feels that >safari's 
>model of one-pinch->one-zooming delta is less problematic for web content than 
>firefox's one-pinch -> more or less zoom depending on the extent of the pinch.  
>It seems to allow for feature discoverability with a fairly innocuous >consequence 
>in the event of occasional triggering, while Firefox can end up with people in a 
>jam quickly.

>I'm not using my regular laptop, so I don't know if you've already addressed >this 
>in nightlies.  If so, ignore!
Comment 30 Henrik Skupin (:whimboo) 2009-03-02 16:56:08 PST
Alex, I run into this issue too several times a day. But I don't think that this is already filed as bug. Looks like we are really sensitive for this action. Do you mind filing a new one?

For now you can clear browser.gesture.pinch.out and browser.gesture.pinch.in inside about:config.
Comment 31 Tom Dyas 2009-03-02 17:02:51 PST
The threshold can be adjusted.  Just increase browser.gesture.pinch.threshold from its current value.
Comment 32 Alex Faaborg [:faaborg] (Firefox UX) 2009-03-02 17:48:10 PST
Filed the follow up bug 481092
Comment 33 Henrik Skupin (:whimboo) 2009-04-15 02:19:17 PDT
Do we need dev-docs for this feature?
Comment 34 Ed Lee :Mardak 2009-05-05 22:13:57 PDT
user-docs could be useful so people know that you can swipe/rotate/pinch to do various actions. And perhaps how they can be configured through prefs (bug 461376)

dev-docs to describe the new mouse event with various Moz*Gesture names and their .direction and .delta. Also how we're not exposing this to web content yet.
Comment 35 Eric Shepherd [:sheppy] 2009-05-13 17:32:26 PDT
Developer documentation has been added:

https://developer.mozilla.org/En/DOM/Mouse_gesture_events
https://developer.mozilla.org/En/NsIDOMSimpleGestureEvent

If someone would look these over and let me know if they have any thoughts, that'd be great.
Comment 36 Chris Ilias [:cilias] 2009-10-29 00:25:01 PDT
user-doc-complete
The article request bug was bug 440628, which is requested during Firefox 3.0, so the article is more geared toward those who are wondering why multi-touch does not work, and the solution being to upgrade.
<https://support.mozilla.com/en-US/kb/Multi-touch+does+not+work?bl=n>

Note You need to log in before you can comment on or make changes to this bug.