If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Touch events are handled with wrong scroll offset after returning back to the previous page

RESOLVED INCOMPLETE

Status

()

Core
Panning and Zooming
RESOLVED INCOMPLETE
3 years ago
2 years ago

People

(Reporter: dmitry.rojkov, Unassigned)

Tracking

Trunk
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sailfish])

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Created attachment 8441324 [details] [diff] [review]
Workaround

How to reproduce:

1. open a Web page (e.g. http://www.gazeta.ru/politics/news/2014/06/17/n_6236541.shtml)
2. scroll it down,
3. click a link (e.g. any link in the left column)
4. go back
5. click on a link avoiding panning or zooming before the click.

Expected outcome: browser open requested link

Actual: browser opens a link which is above the clicked one.

The scroll position is restored all right but touch events seem to have wrong coordinates within the page until the page is scrolled, zoomed or rotated.

I bumped into this bug with Sailfish browser, but it's also reproducible in FFOS emulator built from HEAD.

The attached patch is just a workaround that seems to alleviate the problem in Sailfish browser at least.
I'm unable to reproduce this on a Flame device or in the emulator using recent master code.
(Reporter)

Comment 2

3 years ago
I've tried again on a fresh build of FFOS running under qemu emulator. The problem reproduces consistently, but only on www.gazeta.ru. Interestingly if to return back to the site's frontpage then the bug is not reproducible.
I used the exact STR that you described in comment 0, so it shouldn't have been using the site's frontpage.

Can you enable the APZCTM_LOG at the top of the APZCTreeManager.cpp and the APZC_LOG at the top AsyncPanZoomController.cpp and reproduce the problem with |adb logcat -v time| running? Attach the logcat here and I can take a look at it.
(Reporter)

Comment 4

3 years ago
Created attachment 8442864 [details]
Output of  adb logcat -v time

1. The log starts when I pan a bit and click a link.
2. Then the link is loaded and I press the back button.
3. The previous page is loaded and I click the same link.
4. But the browser loads a different link (which was much above the I clicked)

The site is very heavy and it takes ages to load on emulator so I never wait for the pages to be completely loaded.
Attachment #8442864 - Attachment mime type: text/x-log → text/plain
Sorry for the delay here. I looked through the log and there's a few things that seem off.

At the time of the first tap (when you click on the link the for the first time) the APZC has these metrics:

06-19 11:53:43.206 I/Gecko   (   44): APZC: 0x49a4f000 got a NotifyLayersUpdated with aIsFirstPaint=0: i=(416 15) cb=(0 0 320 361) rcs=(980.000 1105.562) dp=(0.000 -1871.183 1016.000 4754.817) dpm=(0.000 0.000 0.000 0.000) um=0 v=(0.000 0.000 980.000 1105.567) s=(0.000 1871.183) sr=(0.000 0.000 1016.000 4754.817) z(ld=1.000 r=0.327 cr=0.327 z=0.327 ts=1.000) u=(0 0)

Note that the scroll offset is at y=1871.183. Also note the scrollable rect is 1016x4754.817. Also according to the output near this line, the click happens at screen coordinates (22, 93) which maps to (22, 19) of the content area. This is all fine. Later, after you go back and click on the link again, the APZC metrics look like this:

06-19 11:55:29.787 I/Gecko   (   44): APZC: 0x4b561000 got a NotifyLayersUpdated with aIsFirstPaint=0: i=(486 17) cb=(0 0 320 361) rcs=(980.000 1105.562) dp=(0.000 0.000 982.000 2145.000) dpm=(0.000 0.000 0.000 0.000) um=0 v=(0.000 0.000 980.000 1105.567) s=(0.000 0.000) sr=(0.000 0.000 982.000 2145.000) z(ld=1.000 r=0.327 cr=0.327 z=0.327 ts=1.000) u=(0 0)

This is pretty close to the original metrics, but now the scroll offset is at (0, 0) and the scrollable rect is 982x2145. The smaller scrollable rect is quite possibly because the page is still loading and hasn't filled in completely. However the scroll offset being 0 means you should be looking at the top of the page rather than content partway down the page. In fact, if you were at scroll offset y=1871.183 as before, then bottom of the visible area (1871.183 + (361 / 0.327)) = 2975.158 would be way past the bottom of the scrollable rect. Also in this case the click happens at screen coordinates (18, 395) which maps to (18, 321) of the content area. Factoring in the zoom and scroll offset this is going to end up at a different location on the page than the first click.

So from the log it appears as though this second click happens while the page is still loading, before it has jumped down to the restored scroll offset. From your description in comment 0 it sounds like the scroll position is restored before you do the click. I'm not really sure how the content is getting rendered at the new scroll position based on the log.
(Reporter)

Comment 6

3 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5)
> Sorry for the delay here. I looked through the log and there's a few things
> that seem off.

Thanks! And no hurry, this issue is not critical for me at the moment. 
 
> So from the log it appears as though this second click happens while the
> page is still loading, before it has jumped down to the restored scroll
> offset. From your description in comment 0 it sounds like the scroll
> position is restored before you do the click. I'm not really sure how the
> content is getting rendered at the new scroll position based on the log.

When I clicked the scroll offset was neither 0 nor the original 1871.183, but somewhere in the middle and closer to 1871.183. Indeed the page was not fully loaded.

Also it may well be not the same problem as in our embedding actually, because the issue is not always reproducible in B2G, but 10/10 in our browser.

Instead of TabChild we use our own class derived from TabChildBase that happens to ignore updates of scroll offset made in ScrollFrameHelper::ScrollToRestoredPosition() [1] and relays obsolete scroll offset further with APZCCallbackHelper::UpdateRootFrame(). I still need to understand where touch screen coordinates are converted to content coordinates and why the real offset gets clobbered by the obsolete one.

[1] http://dxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#3006
(In reply to dmitry.rojkov from comment #6)
> Instead of TabChild we use our own class derived from TabChildBase that
> happens to ignore updates of scroll offset made in
> ScrollFrameHelper::ScrollToRestoredPosition() [1] and relays obsolete scroll
> offset further with APZCCallbackHelper::UpdateRootFrame(). I still need to
> understand where touch screen coordinates are converted to content
> coordinates and why the real offset gets clobbered by the obsolete one.

That definitely sounds related to this problem. If the APZC isn't aware of these scroll position changes then it's not going to be able to send the tap to the right place.
Whiteboard: [sailfish]
Did you ever figure out what was going on here? Are you still able to reproduce this problem?
Flags: needinfo?(dmitry.rojkov)
(Reporter)

Comment 9

3 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)
> Did you ever figure out what was going on here? Are you still able to
> reproduce this problem?

We got stuck with gecko31 which is ESR now and can't keep up with changes in the engine due to lack of resources. For gecko31 we have to have applied this package patch [1].

I still hope we'll start moving to a newer engine by the end of this month, then I'll have to review all the patches we currently use.

[1] https://github.com/nemomobile-packages/gecko-dev/blob/nemo_embedlite_31/rpm/fix-20430-invalidate-obsolete-scroll-offset.patch
(Reporter)

Updated

3 years ago
Flags: needinfo?(dmitry.rojkov)
Ok, let me know if you still see the problem after updating. Sorry for the continual churn - it probably doesn't make your life any easier :/
I'm going to close this bug since it doesn't look like it's going anywhere at the moment, and we haven't seen it crop up anywhere else. Please feel free to reopen if it's still an issue that needs tracking.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.