Last Comment Bug 775463 - Implement double-tap-to-zoom content
: Implement double-tap-to-zoom content
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics: Layers (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla17
Assigned To: Doug Sherk (:drs) (inactive)
:
: Milan Sreckovic [:milan]
Mentors:
Depends on: 781021 851556
Blocks: 745136 781456
  Show dependency treegraph
 
Reported: 2012-07-19 03:22 PDT by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2013-03-15 10:08 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
?
+


Attachments
Recognize double tap gestures while still supporting single taps (14.61 KB, patch)
2012-07-26 01:25 PDT, Doug Sherk (:drs) (inactive)
cjones.bugs: feedback+
Details | Diff | Splinter Review
Recognize double tap gestures while still supporting single taps (10.61 KB, patch)
2012-07-26 20:37 PDT, Doug Sherk (:drs) (inactive)
bugmail: feedback-
Details | Diff | Splinter Review
Recognize double tap gestures while still supporting single taps (12.81 KB, patch)
2012-07-27 14:56 PDT, Doug Sherk (:drs) (inactive)
cjones.bugs: review+
bugzilla: checkin+
Details | Diff | Splinter Review
Implement double-tap-to-zoom content (45.92 KB, patch)
2012-08-02 17:00 PDT, Doug Sherk (:drs) (inactive)
no flags Details | Diff | Splinter Review
Implement tolerance in double tap detection (7.04 KB, patch)
2012-08-02 17:02 PDT, Doug Sherk (:drs) (inactive)
no flags Details | Diff | Splinter Review
Implement double-tap-to-zoom content (48.52 KB, patch)
2012-08-03 12:18 PDT, Doug Sherk (:drs) (inactive)
no flags Details | Diff | Splinter Review
Implement tolerance in double tap detection (7.04 KB, patch)
2012-08-03 12:18 PDT, Doug Sherk (:drs) (inactive)
cjones.bugs: review+
Details | Diff | Splinter Review
Implement double-tap-to-zoom content (47.12 KB, patch)
2012-08-03 20:21 PDT, Doug Sherk (:drs) (inactive)
no flags Details | Diff | Splinter Review
Implement double-tap-to-zoom content (47.24 KB, patch)
2012-08-07 14:53 PDT, Doug Sherk (:drs) (inactive)
cjones.bugs: review+
Details | Diff | Splinter Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-19 03:22:14 PDT
We need to
 - recognize double-tap gestures
 - send a message to content requesting it to find an appropriate element to zoom at the double-tap focus point
 - reply with the element and its metrics
 - kick off the zoom animation on the compositor and start repainting in content

The heuristics for this have lived in browser.js forever.

Suggesting blocking basecamp because this is pretty much a table-stakes UI feature now in "mobile" browsers.
Comment 1 Doug Sherk (:drs) (inactive) 2012-07-26 01:25:55 PDT
Created attachment 646061 [details] [diff] [review]
Recognize double tap gestures while still supporting single taps

I'm using the compositor thread as a heartbeat signal to the gesture detector. I do this because we need to send a delayed single tap if we detect that a double tap isn't happening. This means that we have to schedule ~500 ms of composites for nothing other than gesture detection. Since there's a lot of overhead to composites, this sounds bad to me. Should we just dispatch a runnable somewhere? Any suggestions?

I will also need to look at and figure out what truly is the difference between SingleTapUp and SingleTapConfirmed, as well as whether or not we even need them. I assume we need them for Fennec, though if it uses its own gesture detector we won't need this implementation. Without them, this code is a lot simpler since we can detect double taps using only actual touch inputs (touch down, touch up, [first tap] touch down, touch up [second tap]).
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-26 02:32:28 PDT
We don't want to use the compositor heartbeat for that --- it's for sampling animations.  We can receive many input events that would result in the same pixels drawn on screen, so turning on the compositor just for timekeeping is quite inefficient.  We only want to turn it on for graphical effects.

You want to post a task delayed by the "can't-be-double-tap" interval.  ISTR that this was 300ms on xul-fennec, but it makes sense to use the same interval that android-fennec is currently using.

This will need to play well with content touch-event listeners when we tackle that problem, but I don't think that adds too much complexity here.

Running out of steam, will review tomorrow.
Comment 3 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-26 07:32:35 PDT
(In reply to Doug Sherk (:dRdR) from comment #1)
> I will also need to look at and figure out what truly is the difference
> between SingleTapUp and SingleTapConfirmed, as well as whether or not we
> even need them. I assume we need them for Fennec, though if it uses its own
> gesture detector we won't need this implementation.

The order of events:
1) user touches down, we get a touchdown
2) user lifts finger, we get a touchup and then a SingleTapUp immediately
3a) user puts finger down again, we get a touchdown
4a) user lifts finger, we get a touchup and then a DoubleTap immediately

OR

1) user touches down, we get a touchdown
2) user lifts finger, we get a touchup and then a SingleTapUp immediately
3b) user does nothing, and <n> ms later, we get a SingleTapConfirmed (n is a system-wide setting on android accessible by ViewConfiguration.getDoubleTapTimeout)

The idea behind fennec responding to both SingleTapUp and SingleTapConfirmed is to avoid input lag - if zooming is disabled by the content then we know that double-tap will do nothing, and we can send the "tap" event as soon as the first tap is done without waiting for the <n>-ms delay.
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-26 09:24:12 PDT
Comment on attachment 646061 [details] [diff] [review]
Recognize double tap gestures while still supporting single taps

Other than the issues above, looks pretty good.

Let's use a PostDelayedTask(NewRunnableMethod(foo, &Bar::M), delay) to do the task management, instead of nsIRunnable.
Comment 5 Doug Sherk (:drs) (inactive) 2012-07-26 20:37:40 PDT
Created attachment 646459 [details] [diff] [review]
Recognize double tap gestures while still supporting single taps

Ok, I fixed it to use PostDelayedTask() (which I completely forgot about) and to do what I understand should be the functionality of TapUp/TapConfirmed based on kats' explanation.
Comment 6 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-27 03:34:05 PDT
Comment on attachment 646459 [details] [diff] [review]
Recognize double tap gestures while still supporting single taps

># HG changeset patch
># Parent fc17a0803716958bcba1ff1b06eb8edfe8302a93
># User Doug Sherk <dsherk2@mozilla.com>
>Bug 775463: Recognize double tap gestures while still supporting single taps
>
>diff --git a/gfx/layers/ipc/GestureEventListener.cpp b/gfx/layers/ipc/GestureEventListener.cpp

>+    if (event.mTime - mTapStartTime <= MAX_TAP_TIME) {
>+      if (mState == WaitingDoubleTap) {
>+        // We were waiting for a double tap and it has arrived.
>+        HandleDoubleTap(event);
>+        mState = NoGesture;
>+      } else {
>+        HandleSingleTapUpEvent(event);

This doesn't work for > 2 touch points right?

>diff --git a/gfx/layers/ipc/GestureEventListener.h b/gfx/layers/ipc/GestureEventListener.h

> protected:
>   enum GestureState {
>+    // There's no gesture going on, and we don't think we're about to enter one.
>     NoGesture = 0,
>-    InPinchGesture
>+    // There's a pinch happening, which occurs when there are two touch inputs.
>+    InPinchGesture,
>+    // A single tap has happened for sure, and we're waiting for a second tap.
>+    WaitingDoubleTap
>   };

Need to be consistent with style here --- local style is NO_GESTURE, etc.

The rest looks OK.  Clearing r? pending answer to question above.
Comment 7 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-27 12:00:08 PDT
Comment on attachment 646459 [details] [diff] [review]
Recognize double tap gestures while still supporting single taps

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

I think there's a race condition that might happen if you tap multiple times in quick succession. Say you do a normal double-tap, which is processed as expected. However there will still be a TimeoutDoubleTap runnable is in the queue waiting to run. Normally the state guard at the top of the function will make it a no-op, but if you tap again once before the timeout expires, then that tap will get cancelled when the timeout expires. The net result is that you want to double-tap twice, but the second double-tap might get cancelled prematurely because of the runnable from the first double-tap. Does that make sense?

Other than that I didn't see any problems.
Comment 8 Doug Sherk (:drs) (inactive) 2012-07-27 14:56:04 PDT
Created attachment 646733 [details] [diff] [review]
Recognize double tap gestures while still supporting single taps

(In reply to Chris Jones [:cjones] [:warhammer] from comment #6)
> This doesn't work for > 2 touch points right?

You're right, there was a big mistake in how I dealt with this. There's now a proper WAITING_SINGLE_TAP state that we enter when there has only been 1 unique touch input since there was last 0 touch inputs.

(In reply to Kartikaya Gupta (:kats) from comment #7)
> I think there's a race condition that might happen if you tap multiple times

Ok, I now cancel the timeout task when a double tap comes in.
Comment 9 Doug Sherk (:drs) (inactive) 2012-07-27 14:59:07 PDT
Comment on attachment 646733 [details] [diff] [review]
Recognize double tap gestures while still supporting single taps

"WaitingDoubleTap" => I will fix references to this in comments before landing.
Comment 10 Doug Sherk (:drs) (inactive) 2012-07-27 17:24:39 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0e33c1c7c17
Comment 11 Doug Sherk (:drs) (inactive) 2012-07-27 17:27:16 PDT
Forgot to fix the comment, followup fix:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d0fb7ac961a
Comment 12 Doug Sherk (:drs) (inactive) 2012-07-27 19:25:52 PDT
Backed out both:
https://hg.mozilla.org/integration/mozilla-inbound/rev/02d63f48e752
Comment 13 Doug Sherk (:drs) (inactive) 2012-07-27 19:26:02 PDT
Pushed again:
https://hg.mozilla.org/integration/mozilla-inbound/rev/db790a34ba9a
Comment 14 Ryan VanderMeulen [:RyanVM] 2012-07-28 18:34:21 PDT
https://hg.mozilla.org/mozilla-central/rev/db790a34ba9a
Comment 15 Doug Sherk (:drs) (inactive) 2012-08-02 17:00:47 PDT
Created attachment 648568 [details] [diff] [review]
Implement double-tap-to-zoom content

Has random stuff scattered inside it, mostly small refactors to support this.
Comment 16 Doug Sherk (:drs) (inactive) 2012-08-02 17:02:11 PDT
Created attachment 648569 [details] [diff] [review]
Implement tolerance in double tap detection

"Implement double-tap-to-zoom content" kind of sucks without this, at least on Otoro. This should make it easy to do a double tap.
Comment 17 Doug Sherk (:drs) (inactive) 2012-08-02 17:12:33 PDT
https://tbpl.mozilla.org/?tree=Try&rev=24ee21078b33
Comment 18 Doug Sherk (:drs) (inactive) 2012-08-02 23:02:02 PDT
(In reply to Doug Sherk (:dRdR) from comment #17)
> https://tbpl.mozilla.org/?tree=Try&rev=24ee21078b33

There's a leak, will deal with this. cjones says I have to add a ClearOnShutdown() to the StaticAutoPtr.
Comment 19 Doug Sherk (:drs) (inactive) 2012-08-03 12:18:19 PDT
Created attachment 648799 [details] [diff] [review]
Implement double-tap-to-zoom content

Unbitrotted
Comment 20 Doug Sherk (:drs) (inactive) 2012-08-03 12:18:55 PDT
Created attachment 648800 [details] [diff] [review]
Implement tolerance in double tap detection

Unbitrotted
Comment 21 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-03 16:02:05 PDT
Comment on attachment 648799 [details] [diff] [review]
Implement double-tap-to-zoom content

>diff --git a/dom/browser-element/BrowserElementGestures.js b/dom/browser-element/BrowserElementGestures.js

This is really a gesture *handler*, just like the current code in
BrowserElementScrolling.js.  Splitting this into a separate file and
giving it an ambiguous name is confusing (I thought originally we were
*computing* gestures here, which made me do a double-take).

>+const ContentGestures = {
>+  init: function cg_init() {
>+    addMessageListener("Viewport:Change", this._recvViewportChange.bind(this));

This is going to process Viewport:Change twice, no?

>+  _recvDoubleTap: function(data) {

Is this code taken from browser.js?  If so, please note that, and I
won't review it.

>diff --git a/dom/ipc/PBrowser.ipdl b/dom/ipc/PBrowser.ipdl

>+    /**
>+     * Instructs the TabParent to forward a request to zoom to a rect given in
>+     * CSS pixels.

Relative to what?  The viewport or document?

>+     */
>+    BrowserZoomToRect(gfxRect aRect);

"Browser" in the name here is redundant, since we're in PBrowser
already ;);.

>+    HandleDoubleTap(nsIntPoint point);
> 

Document this, especially coordinate space.

>diff --git a/dom/ipc/TabChild.cpp b/dom/ipc/TabChild.cpp

>+      int numAssigned = sscanf(NS_ConvertUTF16toUTF8(aData).get(),
>+                               "{\"x\":%lf,\"y\":%lf,\"w\":%lf,\"h\":%lf}",
>+                               &rect.x, &rect.y, &rect.width, &rect.height);

May god have mercy on our souls.

>+void
>+TabChild::WrapAndSendJSON(const char* aTopic, const nsACString& aData)

DispatchMessageManagerMessage(const nsACString& aMessageName, const nsACString& aJSONData)

>diff --git a/dom/ipc/TabChild.h b/dom/ipc/TabChild.h

>+    // XXX: Do the work the browser chrome script does in C++ instead so we
>+    // don't need things like this.
>+    void WrapAndSendJSON(const char* aTopic, const nsACString& aData);

This wants to be a FIXME/bug XXXXXX comment.

>diff --git a/gfx/layers/ipc/AsyncPanZoomController.cpp b/gfx/layers/ipc/AsyncPanZoomController.cpp

>+StaticAutoPtr<css::ComputedTimingFunction> gComputedTimingFunction;

|using namespace mozilla::css;|

>+  if (!gComputedTimingFunction) {
>+    gComputedTimingFunction = new css::ComputedTimingFunction();
>+    gComputedTimingFunction->Init(nsTimingFunction(0, 0, 0.58, 1.0));

Would prefer to use a keyword function like
NS_STYLE_TRANSITION_TIMING_FUNCTION_EASE so that (i) the code is
easier to read; and (ii) the zoom "feels" more like common CSS
animations, instead of a custom animal.


>+void AsyncPanZoomController::ZoomToRect(const gfxRect& aRect) {
>+  mState = ANIMATED_ZOOM;

Needs the mutex, no?

>diff --git a/gfx/layers/ipc/AsyncPanZoomController.h b/gfx/layers/ipc/AsyncPanZoomController.h

>+  /**
>+   * Kicks an animation to zoom to a rect. This may be either a zoom out or zoom
>+   * in. The actual animation is done on the compositor thread after being set
>+   * up. |aRect| must be given in CSS pixels.
>+   */
>+  void ZoomToRect(const gfxRect& aRect);

Relative to what?

>   enum PanZoomState {

>+    ANIMATED_ZOOM   /* animated zoom to a new rect */

ANIMATING_ZOOM

>+  // Old metrics from before we started a zoom animation. This is only valid
>+  // when we are in the "ANIMATED_ZOOM" state. This is used so that we can
>+  // interpolate between the start and end frames. We only use the
>+  // |mViewportScrollOffset| and |mResolution| fields on this.
>+  FrameMetrics mStartZoomToMetrics;

What happens if content repaints in the middle of a zoom?  Should this
always be mLastContentPaintMetrics, and then re-adjust the animation
duration appropriately if a new paint comes in?

>diff --git a/gfx/layers/ipc/GeckoContentController.h b/gfx/layers/ipc/GeckoContentController.h

>+  /**
>+   * Requests handling of a double tap.
>+   */
>+  virtual void HandleDoubleTap(const nsIntPoint& aPoint) = 0;

Needs moar docs.  Coordinate space etc.

>diff --git a/gfx/layers/ipc/ShadowLayerUtils.h b/gfx/layers/ipc/ShadowLayerUtils.h

Why the change here?  Why can't we keep the FrameMetrics serializer in
ShadowLayerUtils.h and include it from PBrowser.ipdl?

>diff --git a/ipc/glue/IPCMessageUtils.h b/ipc/glue/IPCMessageUtils.h
> template<>
>+struct ParamTraits<gfxRect>
>+{

>+  static bool Read(const Message* aMsg, void** aIter, paramType* aResult)
>+  {
>+    if (ReadParam(aMsg, aIter, &aResult->x) &&
>+        ReadParam(aMsg, aIter, &aResult->y) &&
>+        ReadParam(aMsg, aIter, &aResult->width) &&
>+        ReadParam(aMsg, aIter, &aResult->height))

      return (ReadParam(aMsg, aIter, &aResult->x) &&
              ReadParam(aMsg, aIter, &aResult->y) &&
              ReadParam(aMsg, aIter, &aResult->width) &&
              ReadParam(aMsg, aIter, &aResult->height));

This looks good overall, but needs some work.
Comment 22 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-03 16:06:06 PDT
Comment on attachment 648800 [details] [diff] [review]
Implement tolerance in double tap detection

>diff --git a/gfx/layers/ipc/AsyncPanZoomController.h b/gfx/layers/ipc/AsyncPanZoomController.h

>+  /**
>+   * Constant describing the tolerance we use, multiplied by the device DPI,
>+   * before we start panning the screen. This is to prevent us from accidentally
>+   * processing taps as touch moves, and from very short/accidental touches
>+   * moving the screen
>+   */
>+  static const float TOUCH_START_TOLERANCE;

What's it a tolerance of?  The first sentence basically reads as,
"TOUCH_START_TOLERANCE is a tolerance for touch starts" ;).

r=me with better comment
Comment 23 Doug Sherk (:drs) (inactive) 2012-08-03 20:21:16 PDT
Created attachment 648937 [details] [diff] [review]
Implement double-tap-to-zoom content
Comment 24 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-06 11:55:41 PDT
Comment on attachment 648937 [details] [diff] [review]
Implement double-tap-to-zoom content

>diff --git a/dom/browser-element/BrowserElementScrolling.js b/dom/browser-element/BrowserElementScrolling.js

Didn't hear back about provenance of this code, but assuming it's
borrowed from browser.js, so didn't look closely.

>diff --git a/dom/ipc/PBrowser.ipdl b/dom/ipc/PBrowser.ipdl

>+    /**
>+     * Requests handling of a double tap. |point| is in CSS pixels, relative to
>+     * the scroll offset. This message is expected to send back a
>+     * "browser-zoom-to-rect" message using the observer service.
>+     */

It's expected to send a ZoomToRect() message, no?  The observer
service notification is just a hack to get us back into C++.

>diff --git a/dom/ipc/TabChild.cpp b/dom/ipc/TabChild.cpp

>+      int numAssigned = sscanf(NS_ConvertUTF16toUTF8(aData).get(),
>+                               "{\"x\":%lf,\"y\":%lf,\"w\":%lf,\"h\":%lf}",
>+                               &rect.x, &rect.y, &rect.width, &rect.height);
>+      NS_ABORT_IF_FALSE(numAssigned == 4, "Invalid JSON format");

MOZ_ASSERT()

>+void
>+TabChild::DispatchMessageManagerMessage(const char* aTopic, const nsACString& aData)

  DispatchMessageManagerMessage(const nsACString& aMessageName,
                                const nsACString& aJSONData)

Note change to parameter names: you're using observer service
terminology, but this is message manager.

>diff --git a/gfx/layers/ipc/AsyncPanZoomController.cpp b/gfx/layers/ipc/AsyncPanZoomController.cpp

>+    case ANIMATED_ZOOM:

ANIMATING_ZOOM

>+      requestAnimationFrame = requestAnimationFrame || DoFling(aSampleTime - mLastSampleTime);

requestAnimationFrame |= ...;

>diff --git a/gfx/layers/ipc/AsyncPanZoomController.h b/gfx/layers/ipc/AsyncPanZoomController.h

>+  static bool Read(const Message* aMsg, void** aIter, paramType* aResult)
>+  {
>+    if (ReadParam(aMsg, aIter, &aResult->x) &&
>+        ReadParam(aMsg, aIter, &aResult->y) &&
>+        ReadParam(aMsg, aIter, &aResult->width) &&
>+        ReadParam(aMsg, aIter, &aResult->height))
>+      return true;
>+
>+    return false;

return (ReadParam() && ...);

There are several unaddressed review comments here.  Please be sure to
look over them.  Need to see the next version.
Comment 25 Doug Sherk (:drs) (inactive) 2012-08-07 14:53:19 PDT
(In reply to Chris Jones [:cjones] [:warhammer] from comment #21)
> Comment on attachment 648799 [details] [diff] [review]
> Is this code taken from browser.js?  If so, please note that, and I
> won't review it.

That's correct.
 
> >+  // Old metrics from before we started a zoom animation. This is only valid
> >+  // when we are in the "ANIMATED_ZOOM" state. This is used so that we can
> >+  // interpolate between the start and end frames. We only use the
> >+  // |mViewportScrollOffset| and |mResolution| fields on this.
> >+  FrameMetrics mStartZoomToMetrics;
> 
> What happens if content repaints in the middle of a zoom?  Should this
> always be mLastContentPaintMetrics, and then re-adjust the animation
> duration appropriately if a new paint comes in?

I don't understand how this would change it. Could you clarify? I could see a change in the CSS rect during an animated zoom messing it up, which is something I should deal with and will in a follow-up.
Comment 26 Doug Sherk (:drs) (inactive) 2012-08-07 14:53:56 PDT
Created attachment 649823 [details] [diff] [review]
Implement double-tap-to-zoom content
Comment 27 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-07 15:26:04 PDT
(In reply to Doug Sherk (:dRdR) from comment #25)
> (In reply to Chris Jones [:cjones] [:warhammer] from comment #21)
> > Comment on attachment 648799 [details] [diff] [review]
> > >+  // Old metrics from before we started a zoom animation. This is only valid
> > >+  // when we are in the "ANIMATED_ZOOM" state. This is used so that we can
> > >+  // interpolate between the start and end frames. We only use the
> > >+  // |mViewportScrollOffset| and |mResolution| fields on this.
> > >+  FrameMetrics mStartZoomToMetrics;
> > 
> > What happens if content repaints in the middle of a zoom?  Should this
> > always be mLastContentPaintMetrics, and then re-adjust the animation
> > duration appropriately if a new paint comes in?
> 
> I don't understand how this would change it. Could you clarify? I could see
> a change in the CSS rect during an animated zoom messing it up, which is
> something I should deal with and will in a follow-up.

That's what I was referring to.  Please to be filing and FIXME/bug XXXXX'ing here.  Or if it's simpler to handle here we can do that too.
Comment 28 Doug Sherk (:drs) (inactive) 2012-08-07 15:31:22 PDT
Filed bug 781021.
Comment 29 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-07 18:15:36 PDT
Comment on attachment 649823 [details] [diff] [review]
Implement double-tap-to-zoom content

I like this patch :).
Comment 31 Phil Ringnalda (:philor) 2012-08-07 21:07:52 PDT
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/d8878c0bd67c - something in that push made Linux oddly unhappy.
Comment 33 Jared Wein [:jaws] (please needinfo? me) 2012-08-08 10:21:58 PDT
A changeset was checked in that referred incorrectly to bug 777463. I believe that it meant to refer to this bug (775463). This is the changeset:
https://hg.mozilla.org/mozilla-central/rev/8b3b879bc63f
Comment 34 Doug Sherk (:drs) (inactive) 2012-08-08 11:02:59 PDT
This was backed out.
Comment 36 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-08 22:49:02 PDT
\o/
Comment 37 Doug Sherk (:drs) (inactive) 2012-08-09 01:24:57 PDT
This needs additional work to be done correctly (right now the animation is weird). I have filed bug 781456 for this.

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