494715 - When panning the content and switching tab, viewport can be wrong

Status: VERIFIED FIXED

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090522 Minefield/3.6a1pre
Build Identifier: 

When panning to the bottom of some website and going to another tab I see only the checkerboard, or depending on the amount panned a part of the web site and the checkerboard

Reproducible: Always

Steps to Reproduce:
1.Open a new tab
2.go to http://www.malaysiabest.net/2005/09/27/the-soup-that-guarantees-to-make-you-burppp/
3.pan to the bottom
4. Switch back to the previous tab
Actual Results:  
The canvas area show the checkerboard

Expected Results:  
the website !

When we click on a tab, the ChromeInputModule of InputHandler catch the click and start a dragging session this prevent the call to viewportUpdate that put the right info to the inner struct of WidgetStack.
Manually calling ws.dragStop() in browser.js#selectedTab correct the problem.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch that call ws.dragStop()

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Update patch

Sorry, I don't want to put ws.dragStop in the try/catch (there is no reason)

Comment 3

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

(In reply to comment #0)
> When we click on a tab, the ChromeInputModule of InputHandler catch the click
> and start a dragging session this prevent the call to viewportUpdate that put
> the right info to the inner struct of WidgetStack.
> Manually calling ws.dragStop() in browser.js#selectedTab correct the problem.

Can we fix the ChromeInputHandler? I'd rather fix the real problem.

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #3)
> Can we fix the ChromeInputHandler? I'd rather fix the real problem.

Yep!

It seems that there is two fix depending on the role of ws.dragStart in the ChromeInputModule :
  // grab all events until we stop the drag
  ws.dragStart(sX, sY);

Firstly the comment seems to be not relevant (has moved between revision and the code probably came from the panning module), and I'm not sure there is any needs for calling the ws.startDrag on chrome input event? 
If that true we can simply remove this the two lines (comment and function call) and this simple fix resolve the problem (We're not anymore considered in a dragging state for the ws and viewportUpdate is called).

Otherwise if the called to dragStart is needed ( for what? ), balanced him in _dragStop (InputHandler.js#288) correct the problem but can lead to quickly see the checkerboard.

Comment 5

[Ben Combee (:bcombee)]

There are other patches in my own queue that also change this code.  I'd like to see those finished and landed and then retest this bug.

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Ben, any news on the patches from your queue?

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Remove ws.dragStart

Comment 8

[YUKI "Piro" Hiroshi]

Attachment: patch v1

Because endUpdateBatch() doesn't update the viewport even if "aForceRedraw" is specified, the base position of the viewport box (#canvasbox) moves unexpectedly.

Comment 9

[YUKI "Piro" Hiroshi]

This blocks bug 476703.

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #8)
> Created an attachment (id=383170) [details]
> patch v1
> 
> Because endUpdateBatch() doesn't update the viewport even if "aForceRedraw" is
> specified, the base position of the viewport box (#canvasbox) moves
> unexpectedly.

Yes. It's what I've said above, we're in a dragging session, so viewportUpdate is not called.

Comment 11

[Ben Combee (:bcombee)]

I saw a different version of this problem with picking pages from the awesome bar using the touch/mouse.  If I went to my iframes test page, panned down, then went to google news, I'd get my viewport completely removed.

Comment 12

[Ben Combee (:bcombee)]

Comment on attachment 381066 [details] [diff] [review]
Remove ws.dragStart

This change makes a lot of sense.  When we're dragging in chrome, we're not going to ever make calls to ws.panBy or anything to reposition the canvas, and this dragStart won't be correctly paired with dragEnd.  Nothing in chrome handling has any other ws calls.

Comment 13

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

Comment on attachment 383170 [details] [diff] [review]
patch v1

Let's try this with the "remove ws.dragStart" patch first.

Comment 14

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

pushed http://hg.mozilla.org/mobile-browser/rev/94b547d119a9

Comment 15

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

Had some checkin problem

backout: https://hg.mozilla.org/mobile-browser/rev/fdd6139cb846
push: https://hg.mozilla.org/mobile-browser/rev/9de9d605a6b1

Comment 16

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OSX 10.5; en-US; rv:1.9.2a2pre)
Gecko/20090808 Fennec/1.0b3pre

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a1pre) Gecko/20090820
Fennec/1.0b3pre