Julien Wajsberg [:julienw] Reporter
	Description • 10 years ago

Since we enabled APZC, it looks like calling "tap" with marionette does not tap at the correct element if we scroll the window (with scrollIntoView for example).

	Malini Das [:mdas] - Away, not checking bugmail
	Comment 1 • 10 years ago

(In reply to Julien Wajsberg [:julienw] from comment #0)
> Since we enabled APZC, it looks like calling "tap" with marionette does not
> tap at the correct element if we scroll the window (with scrollIntoView for
> example).

Do you have a test case or some STR? I'd like to test this. I tried to test this in Settings, but calling scrollIntoView on one of the settings options just gave me a white screen...

	Julien Wajsberg [:julienw] Reporter
	Comment 2 • 10 years ago

I have a testcase using marionette JS.

You can take my patch in https://github.com/mozilla-b2g/gaia/pull/22349, revert (for example) the change in apps/verticalhome/test/marionette/app_packaged_resume_test.js, and try to run this test using this command (from the gaia directory):

  TEST_FILES=apps/verticalhome/test/marionette/app_packaged_resume_test.js bin/gaia-marionette

	Julien Wajsberg [:julienw] Reporter
	Comment 3 • 10 years ago

Mmm maybe better to revert a change that has scrollIntoView, for example the change to  apps/verticalhome/test/marionette/app_uninstall_test.js

	Malini Das [:mdas] - Away, not checking bugmail
	Comment 4 • 10 years ago

Thanks, I'll take a look at this soon

	Julien Wajsberg [:julienw] Reporter
	Comment 5 • 10 years ago

The patch in previous PR just landed.

I can probably try to do a failing case taking one of the case I had if you want?

	Malini Das [:mdas] - Away, not checking bugmail
	Comment 6 • 10 years ago

(In reply to Julien Wajsberg [:julienw] from comment #5)
> The patch in previous PR just landed.
> 
> I can probably try to do a failing case taking one of the case I had if you
> want?

If the steps in Comment #2 still work with the pushed changes, then it should be fine. If not, then a new case would be helpful.

	Julien Wajsberg [:julienw] Reporter
	Comment 7 • 10 years ago

I added a test that shows the issue.

I run it using from Gaia using:
  DISPLAY=:1 TEST_FILES=apps/verticalhome/test/marionette/scroll_tap_test.js bin/gaia-marionette

(I have a Xephyr running as my :1 display)

As a side note, if I remove the "scroll" part, I can see the homescreen is still scrolled (you can check this by changing the parameter to "getConfirmDialog" so that it doesn't find it anymore, and clicking "cancel" manually).

Maybe calling "tap" scrolls the view in marionette? Or in our helper?

	Zac C (:zac)
	Comment 8 • 10 years ago

Can you be more accurate in the subject? If Marionette cannot find the element then it will raise an ElementNotFoundException.

but if it doesn't tap on the correct location then that's different.

	Julien Wajsberg [:julienw] Reporter
	Comment 9 • 10 years ago

Actually, before we used "click" instead of "touchstart+touchend" and it worked fine.

But:
* when we used "click" with marionette, maybe it uses the element's DOM click method? In that case, marionette doesn't even try to find the correct location?
* when we used "tap" with marionette, then the real "click" event is probably dispatched after ~50ms. So maybe that's about the time for the scroll to be "really" finished.

So maybe it's a timing issue after all:
* we call scrollIntoView
* we call "tap" right after
=> the situation is not really stable, especially with APZC where scrolling happens in a separate thread. So maybe the coordinates that marionette gets are not the ones where the element is _yet_.

	Julien Wajsberg [:julienw] Reporter
	Comment 10 • 10 years ago

What I'd like to try here: take the various tests where I had to add the workaround, and just sleep some 100ms after calling "scrollIntoView" to let gecko settle. I don't know when I'll have time to do this though...

	Julien Wajsberg [:julienw] Reporter
	Comment 11 • 10 years ago

(In reply to Julien Wajsberg [:julienw] from comment #10)
> What I'd like to try here: take the various tests where I had to add the
> workaround, and just sleep some 100ms after calling "scrollIntoView" to let
> gecko settle. I don't know when I'll have time to do this though...

From Bug 1094076 comment 18: sleeping after calling "scrollIntoView" works fine. Sleeping about 600ms seems to be enough.

We should try to find the root cause though ;)

	David Burns :automatedtester
	Comment 12 • 8 years ago

Closing out B2G related bugs. If these still happen and are valuable, please reopen with details again and how this affects Desktop/Fennec