Closed Bug 599220 Opened 10 years ago Closed 10 years ago

[Regression] Panning is jumpy in local pages

Categories

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

defect
Not set

Tracking

(fennec2.0b1+)

VERIFIED FIXED
Tracking Status
fennec 2.0b1+ ---

People

(Reporter: mfinkle, Assigned: mfinkle)

Details

Attachments

(1 file)

The "scroll" event is fired for local pages, causing code to be fired that probably should not be fired. Remote pages do not fire "scroll" events due to the shadow layer scrolling system.

Caused by bug 598391
tracking-fennec: --- → 2.0b1+
Attached patch patchSplinter Review
Uses Ben's idea to move the "scroll" message handler into the remote browser binding. In the process, I moved the receiveMessage handler into a <field> object so we didn't need to completely re-implement the <method> version.

I didn't see any JS errors in console or debug output. Also, Bug 598391 did not seem to regress. Scrolling about:license also seemed smoother.
Assignee: nobody → mark.finkle
Attachment #478167 - Flags: review?(webapps)
Comment on attachment 478167 [details] [diff] [review]
patch

fixing about:license problem for me, thanks
Attachment #478167 - Flags: feedback+
Comment on attachment 478167 [details] [diff] [review]
patch

>+      <field name="_messageListenerRemote"><![CDATA[
>+            switch (aMessage.name) {
>+              case "scroll":
>+                // Cache viewport coordinates are specified relative to CSS viewport, so we
>+                // have to be sure we send the message.
>+                self._pendingPixelsX = Number.MAX_VALUE;
>+                self._pendingPixelsY = Number.MAX_VALUE;
>+

Now that I've read this again, my comment is hard to understand. Maybe something like:

// When CSS scroll offset changes, we must redefine our cache viewport because
// the cache viewport coordinate system's origin is the CSS scroll offset. Setting
// _pendingPixels* guarantees that _updateCacheViewport is called in scrollTo.

Is that more clear? I find this stuff hard to describe sometimes.

r+!
Attachment #478167 - Flags: review?(webapps) → review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/48fb7360d36d
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
verified FIXED on build:

Mozilla/5.0 (Maemo; Linux armv71; rv:2.0b6pre) Gecko/20100927 Namoroka/4.0b7pre Fennec/2.0b1pre

and

Mozilla/5.0 (Android; Linux armv71; rv:2.0b6pre) Gecko/20100927 Namoroka/4.0b7pre Fennec/2.0b1pre


Follow-up bug 599873 filed.
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
OS: Mac OS X → All
Hardware: x86 → All
You need to log in before you can comment on or make changes to this bug.