717844 - implement scrolling for B2G

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Andreas Gal :gal]

I ported scrollability to B2G:

http://people.mozilla.org/~gal/tableview.html

Its a super ugly hack and needs to be cleaned up, but we should be able to do our B2G scrolling based on this. Chris, I think we should start with scrolling all iframes with this. If we want multiple scroll containers in a webapp, or sub-page scrolling, we just use an inner iframe (contacts with a menu on top for example). What do you think?

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I think we could inject a frame script into apps and apply the scrollability heuristics as the default action.  This is heading down the road that fennec took, which I don't want to do, but realistically it's going to be a big challenge to get kinetic scrolling the platform before demophone.

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I thought this feature is already exists in the gecko binary? If not, how does Firefox Mobile scroll?

If there isn't, I don;t think we should "inject" the <script> into the frames; I think the proper way to do this is to simulate scrolling in chrome scripts, e.g. extend touch.js (vingtetun's work)

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TW) from comment #2)
> I thought this feature is already exists in the gecko binary? If not, how
> does Firefox Mobile scroll?
> 

Kinetic scrolling, sadly no.  Fennec (XUL, old frontend) uses a combination of frame scripts and a special interface, http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsIFrameLoader.idl#114 .

> If there isn't, I don;t think we should "inject" the <script> into the
> frames; I think the proper way to do this is to simulate scrolling in chrome
> scripts, e.g. extend touch.js (vingtetun's work)

Frame scripts (rather poorly named) are https://developer.mozilla.org/en/The_message_manager .  They're purely chrome; there's no injection into page content.  I'm happy to extend what vingtetun built with the heuristics here.

Comment 4

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

I vote for the frame/content script injection.

Comment 5

[Andreas Gal :gal]

Vivien, whats the status here? We urgently need this.

Comment 6

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

(In reply to Andreas Gal :gal from comment #5)
> Vivien, whats the status here? We urgently need this.

I was lurking into other part of the code but I would attach a patch ASAP.

Comment 7

[Andreas Gal :gal]

<3

Comment 8

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

Attachment: wip-1

This wip implements a basic kinetic panning for B2g (scroll div / iframes / browser). This is quite fast on desktop but slow on device obviously.

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

What do you mean specifically by "slow on device"?  What's slow, in what app?

It's not obvious that this should be slow! ;)

Comment 10

[Andreas Gal :gal]

Yeah why is it slow? Need to fix that.

Comment 11

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

(In reply to Andreas Gal :gal from comment #10)
> Yeah why is it slow? Need to fix that.

I use scrollBy/[scrollTop/scrollLeft] that cause a reflow everytime there is an update.

In terms of performance, using mozTransform on the device give much better results. My main problem beeing that not all the scrollable list is visible so this doesn't work. I can probably do a some magics with 'screenshots' but I'm not sure we want to go so far (e.g taking a screenshot of the application, and having the full image of the scrollable list and move it only - taken with something similar to http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/canvas/nsIDOMCanvasRenderingContext2D.idl#269)

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

So, let's focus on getting something landed ASAP.  Slow scrolling is infinitely better than no scrolling, and we can parallelize the optimizations after this code lands.

There are lots of fixes for what you observe, but let's tackle that in followup(s).

Comment 13

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

Attachment: patch

This is the basic version of the patch (slow on device) that active kinetic panning for 'everything'.

The patch still need to be sure everything is fine with the touch events that has landed but this should be only small changes.

Comment 14

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 593564 [details] [diff] [review]
patch

>diff --git a/b2g/chrome/content/panning.js b/b2g/chrome/content/panning.js

>+const ContentPanning = {
>+  init: function cp_init() {
>+    ['mousedown', 'mouseup', 'mousemove'].forEach(function(type) {

We have real touch events on device now.  We should listen to those.
But see below.

>+ContentPanning.init();
>+

This is hard-coded for single process.  We're very soon going to have
multi-process <iframe browser>.  I'll r+ this so we can start tuning
the panning perf in parallel, but this needs to be ported to
framescript asap.

I don't think much of the impl will need to change.

>+const KineticPanning = {

I didn't review this very closely since, based on what you report,
it'll be very much in flux for a while.

>diff --git a/b2g/chrome/content/touch.js b/b2g/chrome/content/touch.js

>-            evt.preventDefault();
>-            evt.stopPropagation();
>+            //evt.preventDefault();
>+            //evt.stopPropagation();

Just remove these.

r=me but we need two immediate followups

 - port to framescript

 - use different panning approach.  Andreas ported scrollability to
   Gecko, let's try that next.  It uses CSS transforms to avoid
   expensive scroll offset munging.

justindarc also made a scrolling lib that we can try.

These can and should happen in parallel.

Comment 15

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/29a26b579a1c

I've got rid of panning.js and move it's code inside a frame script.

For better performance the followup bug is bug 723373

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/29a26b579a1c