604099 - Panning prevented - this._dragger is null

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Panning/Zooming, defect)

Description

[Josh Matthews [:jdm]]

Visited news.google.com, zoomed in and then performed a bunch of minute pans around the chrome border while experimenting with snapping.

Comment 1

[Josh Matthews [:jdm]]

Mouse clicks also yield the same error.

Comment 2

[Josh Matthews [:jdm]]

Zooming still works, but I've lost all ability to manipulate page content.  Refreshing doesn't fix it, and changing pages also doesn't improve the situation.  This should probably block, since this is a serious issue.

Comment 3

[Josh Matthews [:jdm]]

STR: Zoom in, pan down, zoom out.

Comment 4

[Benjamin Stover (:stechz)]

This is not reproducing for me. Could you try on latest nightly?

Comment 5

[Josh Matthews [:jdm]]

Yep, I can still repro this really easily on the latest mac nightly.

Comment 6

[Benjamin Stover (:stechz)]

What is the "zoom in" step? Double tap?

Comment 7

[Josh Matthews [:jdm]]

Pinch zoom on my MBP.

Comment 8

[Matt Brubeck (:mbrubeck)]

I can reproduce this on Linux desktop:
1. Launch Fennec.
2. Go to http://xkcd.com/ (or any zoomable page, as far as I can tell)
3. Press control-shift-p.
4. After the animation finishes, try to pan.

Comment 9

[Benjamin Stover (:stechz)]

Attachment: Panning prevented - this._dragger is null

Comment 10

[Benjamin Stover (:stechz)]

Comment on attachment 484092 [details] [diff] [review]
Panning prevented - this._dragger is null

Matt and I had talked about this bug but I forgot to fix it. doDragStop needs to make sure that we are dragging first.

Comment 11

[Matt Brubeck (:mbrubeck)]

I missed a step in comment 8: after step 2, open the left sidebar (tab controls).

Comment 12

[Matt Brubeck (:mbrubeck)]

Comment on attachment 484092 [details] [diff] [review]
Panning prevented - this._dragger is null

This fixes the problem.

I feel like there's still a fragile and unstated invariant in this code, though.  Would it be reasonable to wrap both of the "this._dragger = null;" statements in "if (this._dragger)" blocks to prevent this from cropping up again?  Or is that likely to indicate a problem that'll be worse if we don't catch it?

Comment 13

[Benjamin Stover (:stechz)]

If those errors didn't occur, we'd be calling dragStop when we really shouldn't be. That makes me nervous. It also makes me think input.js should cry loudly when things aren't as it expects (or: unit tests!).

Comment 14

[Mark Finkle (:mfinkle) (use needinfo?)]

browser-chrome tests are welcome

Comment 15

[Benjamin Stover (:stechz)]

Pushed http://hg.mozilla.org/mobile-browser/rev/4e579c4df89b

Comment 16

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I had repro'ed this on Android yesterday.

Verified w/ today's build that this is fixed:
Mozilla/5.0 (Maemo;Linux armv71; rv:2.0b8pre) Gecko/20101019 Firefox/4.0b8pre Fennec/4.0b2pre
Mozilla/5.0 (Android; Linux armv71; rv2.0b8pre) Gecko/20101019 Firefox/4.0b8pre Fennec/4.0b2pre
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101019 Firefox/4.0b8pre Fennec/4.0b2pre