487485 - Canvas snaps back to top of page at end of page load even after user pans away

Status: VERIFIED FIXED

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

Description

[Ben Combee (:bcombee)]

A change in CanvasBrowser.js causes the endLoading event to pan the page back to 0,0 and end of loading.  This needs to be fixed.

Comment 1

[Ben Combee (:bcombee)]

Attachment: Fix snap-to-top happening on endLoading by doing it earlier if there are pans

Comment 2

[Ben Combee (:bcombee)]

Attachment: Update for bitrot after recent canvasbrowser changes

This applies cleanly over my CanvasBrowser cleanup patch now

Comment 3

[Ben Combee (:bcombee)]

OK, I just validated that this fixed Planet Mozilla on Win32 desktop... without this, the page would jump back to top when it finished loading, even if I'd scrolled down.  With this, I don't get that snap back.

I need to update this slightly to change panTo(0,0) to be panTo(0,-60) to avoid not showing the toolbar while loading a new page.  That's coming right now.

Comment 4

[Ben Combee (:bcombee)]

Attachment: update panTo location to match what's needed

Comment 5

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

Comment on attachment 374373 [details] [diff] [review]
update panTo location to match what's needed

I don't think we even need to keep the "needToPanToTop" test in CanvasBrowser.zoomPage. The new check you add seems to always get called first.

I don't like "needToPanToTop" and I don't want to build more on top of it, if possible. 

I have an alternative that removes "needToPanToTop" altogether. Together with the patch in bug 484234, the results seem to be quite good.

Comment 6

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

(In reply to comment #5)
> (From update of attachment 374373 [details] [diff] [review])

> I have an alternative that removes "needToPanToTop" altogether. Together with
> the patch in bug 484234, the results seem to be quite good.

Trickier than I hoped. Also, the panTo itself is sort of expensive (takes time), so trying to piggy back it into a batch update, as Taras had done, as a good goal.

My current WIP is not in a batch, so I'll try to find a way to get it in a batch.

Comment 7

[Stuart Parmenter]

this is fixed now

Comment 8

[Aakash Desai [:aakashd]]

verified FIXED on builds:

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

and

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