Due to the caching of browser output in fennec to enable fast panning and zooming, the browser will in some cases give wrong values to scripts running on web pages. An example is when you show a web page and then scroll one line down, the top of the page will still be "visibel" as far as the browser is concerned, hence scrollTop will still contain 0 - but the top will not be visble to the end user, as we have just scrolled down one line. For description of scrollTop see: https://developer.mozilla.org/en/DOM/element.scrollTop For description of scrollLeft see: https://developer.mozilla.org/en/DOM/element.scrollleft For an introduction to the panning and zooming in fennec see: https://wiki.mozilla.org/Mobile/Fennec/Architecture#Panning.2FZooming Another issue to consider here is that a script might be able to get the visible vindow size (the size of the "browser" window), we need to verify that this is reported correctly as the size of the fennec window. And if not, to fix it. One interface a script might use can be found here: http://dev.w3.org/csswg/cssom-view/

This bug is the root cause of many bugs we currently have and some of them looks formfill related but appears because the underlying engine is also waiting for right scrollTop/scrollLeft positions. This patch sync the browser and the content view and try to avoid as much as possible the perf impact by ignoring all resulting scroll/MozAfterPaint events. I still need to set up a good test for measuring performances impact of thus type of change. One of Mark's suggestion is to use xresponse to simulate mousedown, mousemove, mouseup and measuring the perfs.

From IRC: vingtetun: a normal build vs a build with patch in bug 479862 give me a perf regression of -.6700% (which can be noise i guess) This test was made using xresponse to "time" the panning performance.

There is a testcase in bug 553193

Comment on attachment 428336 [details] [diff] [review] wip >diff -r d61fdff66712 chrome/content/browser.js > dragStop: function dragStop(dx, dy, scroller) { >- this.bv.resumeRendering(); >+ // Sync browser with view and ignore resulting paint/scroll events >+ let bv = this.bv; >+ Util.executeSoon(function() { >+ bv.ignorePageScroll(true); >+ bv.ignorePagePaint(true); >+ }); >+ Browser.scrollBrowserToContent(); >+ Util.executeSoon(function() { >+ bv.ignorePagePaint(false); >+ bv.ignorePageScroll(false); >+ }); >+ >+ bv.resumeRendering(); This doesn't seem right. By using executeSoon, you essentialy get this for code execution: Browser.scrollBrowserToContent(); // executed right away bv.ignorePageScroll(true); // delayed by first executeSoon bv.ignorePagePaint(true); // delayed by first executeSoon bv.ignorePageScroll(false); // delayed by second executeSoon bv.ignorePagePaint(false); // delayed by second executeSoon

From the duplicate bug: 1. Open http://www.hskupin.info/photos/ 2. Scroll down and click on one of the images An overlay will open and the image is displayed way up the page so you have to scroll a lot. Instead the image should be displayed in the area which is currently visible. That's the way how it works in Firefox.

If this bug is important, and I think it is, we should start working on it. Ben, if you can't start on it, we need to reassign.

Sure. While we are doing this, it would be nice to just set displayport and leave it alone. Instead of updating displayport, we should update the content scroll offset.

we need a plan for this for beta 3

Given the dependencies, this won't land for b3

This is an updated patch based loosely on the previous WIP. I do see some of the gray lines mentioned in bug 612599 with this patch active.

The website in comment 8 works well with this patch, except for the gray lines.

Vivien - Can you pick this up?

(In reply to comment #16) > The website in comment 8 works well with this patch, except for the gray lines. Well, there is also a problem where the scroll snaps back to the top. I think I need to ignore a "scroll" event.

This patch seems to fix the original problem and also not break left/right panning too.

Same as before, but now we check to make sure the "Content:ScrollTo" / "Content:ScrollBy" message will actually cause a "scroll" event to be fired. If we don't the ignoreScroll flag will be out of sync.

Comment on attachment 505065 [details] [diff] [review] patch 3 >diff --git a/chrome/content/bindings/browser.js b/chrome/content/bindings/browser.js > case "Content:ScrollBy": >+ if (!json.dx && json.dy) >+ return; >+ I think you mean if (!json.dx && !json.dy) ? r+ with comments addressed. This is so good to see this bug fixed!

(In reply to comment #21) > > case "Content:ScrollBy": > >+ if (!json.dx && json.dy) > >+ return; > >+ > > I think you mean if (!json.dx && !json.dy) ? Yep! pushed with that change: http://hg.mozilla.org/mobile-browser/rev/1c390a1b9699

I need to update the cache viewport too, or we are left with a big gray block after kinetic panning stops.

Comment on attachment 505110 [details] [diff] [review] followup patch FWIW this could be jerky because CSS scroll offset is changed and then displayport message is processed. This should probably be done in one message if I'm right.

pushed: http://hg.mozilla.org/mobile-browser/rev/ae4e1645e425 (ben mentioned on IRC that he saw no jerky jiggle)

Verified: Mozilla/5.0 (Android; Linux armv71; rv2.0b10pre) Gecko/20110120 Firefox/4.0b10pre Fennec/4.0b4pre

Something (possibly this patch) seems to have regressed scrolling by clicking on in-page links (URI fragments), for example: http://people.mozilla.com/~mbrubeck/test/in-page-link.html

(In reply to comment #27) > Something (possibly this patch) seems to have regressed scrolling by clicking > on in-page links (URI fragments), for example: > http://people.mozilla.com/~mbrubeck/test/in-page-link.html See bug 627500.

regressed by bug 627500

fixed by patch in bug 627500

Following the steps given in comment 8 I was not able to reproduce it, marking it as verified fixed. Mozilla /5.0 (Android;Linux armv7l;rv:6.0a1) Gecko/20110517 Firefox/6.0a1 Fennec/6.0a1 Device: LG Optimus 2X (Android 2.2)