Closed Bug 1174532 Opened 9 years ago Closed 9 years ago

[Aries] Very hard to tap elements in search result on Voyages-SNCF.mobi

Categories

(Core :: Panning and Zooming, defect, P2)

Product:
Core
Component:
Panning and Zooming
Platform:
ARM
Gonk (Firefox OS)
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla45
Project Flags:
blocking-b2g 2.5+
tracking-b2g backlog
Tracking Flags:
Tracking Status
firefox45 --- fixed
b2g-v2.5 --- verified
b2g-master --- verified

People

(Reporter: gerard-majax, Assigned: kats)

References

Details

(Keywords: dogfood, foxfood)

Attachments

(3 files)

1174532.htm - demo, try tapping the link with a slightly moving finger
909 bytes, text/html
Details
cherry-pick
1.60 KB, text/plain
Details
MozReview Request: Bug 1174532 - Even if the APZC is not pannable, have a small slop area to consume touchmove events. r?botond
40 bytes, text/x-review-board-request
botond
: review+
mpotharaju
: approval‑mozilla‑b2g44+
Details 
STR:
 0. Open http://www.voyages-sncf.mobi website
 1. Focus in Departure or Arrival
 2. Tap a station name, like Paris
 3. Wait for the propositions to be loaded and displayed

Expected:
 When I tap a station name, it's being selected

Actual:
 I'm unable to tap any station name

This is working on Firefox for Android, Nightly and Aurora. This is not working on B2G
Kats, is it possible APZC or layers are being funny here?
Flags: needinfo?(bugmail.mozilla)
And make sure you have user agent override:
> general.useragent.override.voyages-sncf.mobi "Mozilla/5.0 (Android; Mobile; rv:41.0) Gecko/41.0 Firefox/41.0"
And that is not reproducing on a Flame, so maybe linked with the higher DPI of my Z3 Compact ?
And I'm starting to wonder if it's possible that bug 1170357 is just a dupe indeed and that the touchmove behavior there is the key?
Summary: Unable to tap elements in search result on Voyages-SNCF.mobi → Very hard to tap elements in search result on Voyages-SNCF.mobi
The touch events are getting preventDefault'ed by the content. Looking in WebIDE I see a touchmove listener on the body element which does have code to call preventDefault. However it's minified/obfuscated so it's not clear to me why it's doing that. I also can't set a breakpoint in WebIDE because it gives me an error "Error loading this URL: http://voyages-sncf.mobi/js/autoComplete.min.20150331082533233.js;jsessionid=78B02D6F9AB8051D3B0B6067873ADD45.bk-vsc-tgv-3"
Component: General → Mobile
Flags: needinfo?(bugmail.mozilla)
Product: Firefox OS → Tech Evangelism
Right, touchmove. Kats, do you mind checking if it's the same reason why we have touchmove in bug 1170357 ? I was not able to find where/how we triggeer touchmove events, and I'd like to make sure we do not have a platform bug here exposed on the Z3 Compact :)
Flags: needinfo?(bugmail.mozilla)
Looks like bug 1170357 already has a patch. The bug here isn't that we're getting a touchmove, that's just because the hardware is more sensitive. The bug is that the JS code isn't dealing with it properly. APZ doesn't do any filtering of touchmove events in this case, so whatever the hardware sends us is what gets passed through. We could change that (bug 1141127), probably, to fix this issue and be more in line what the JS code was written to expect. However then you'd end up increasing latency and also sacrificing precision in other scenarios like games and stuff, because APZ would eat some touchmove events that the games are relying on. So either way we have a problem, it's a question of which is less bad.

I think in the long run the right answer is to keep doing what we're doing now (i.e. pass through all the hardware events to the content) and finish the touch-action implementation so that for code like this website, they don't need to use preventDefault to stop panning, they can just declare it directly using touch-action.
Flags: needinfo?(bugmail.mozilla)
Oh I guess another option here might be to change the implementation so that if a touchstart is prevented we keep the current behaviour of not using that touch block for anything, but if a touchmove is prevented we still allow taps and not panning. I think Chrome does this - but again, it might break stuff if content is written to expect things a certain way. The way the spec is written it's not really clear which is the right thing to do.
Right. I have very few confidence in them providing a fix. Given we may have a workaround that mimics Chrome's behavior (I dislike it, but Chrome is the new IE, right?) and given this may well be affecting other contents, are we sure we do not want to apply it?

Sorry to insist, but this is the kind of dogfooding blocker bugs.
Flags: needinfo?(bugmail.mozilla)
Keywords: foxfood
What is the process for prioritizing foxfooding bugs? We can investigate doing something to fix this but it means I will have less time to work on other stuff so I need to know what is more important.
Flags: needinfo?(bugmail.mozilla)
I experience this issue on the voyages-sncf website but also in plenty of other places like tapping any link in emails in the fastmail mobile website (well, I assume it's the same issue but I didn't actually check for touchmove events and whatnot).
I wonder if that compat bug wouldn't be related: https://webcompat.com/issues/1258

I couldn't tap on "skip" in the Yahoo! login process.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)
> Oh I guess another option here might be to change the implementation so that
> if a touchstart is prevented we keep the current behaviour of not using that
> touch block for anything, but if a touchmove is prevented we still allow
> taps and not panning. 

That sounds like something I would expect as a web developer or script author - +1.
Kats: do you want to move this back to core to sort this out (hope the attached TC shows the issue clearly)?
Flags: needinfo?(bugmail.mozilla)
Alexandre, I had no problems selecting cities on the Flame now. Do you still see the problem?
Flags: needinfo?(lissyx+mozillians)
(In reply to Hallvord R. M. Steen [:hallvors] from comment #15)
> Alexandre, I had no problems selecting cities on the Flame now. Do you still
> see the problem?

This was never a problem on the Flame. Only high end devices with good screen: Z3 Compact for instance. Confere comment 2.
Flags: needinfo?(lissyx+mozillians)
Yup, moving back to APZ-land. Fixing this may be a bit involved though so I wouldn't expect a fix too soon.
Component: Mobile → Panning and Zooming
Flags: needinfo?(bugmail.mozilla)
Product: Tech Evangelism → Core
> This was never a problem on the Flame.

Ah, sorry - I misunderstood. I guess I don't have any device for testing it, but you could verify that the test case I wrote behaves much the same way as the site on this device?
(In reply to Hallvord R. M. Steen [:hallvors] from comment #18)
> > This was never a problem on the Flame.
> 
> Ah, sorry - I misunderstood. I guess I don't have any device for testing it,
> but you could verify that the test case I wrote behaves much the same way as
> the site on this device?

Loaded the page, I cannot tap the link.
And the same attachment loaded in Firefox for Android (nightly from yesterday) on the same device works well.
Might also be the same problem that makes it very hard to use the France 24 app on B2G on the Z3 Compact device ...
Flags: needinfo?(hsteen)
Flags: needinfo?(bugmail.mozilla)
Please flag as tracking if you want this prioritized. At the moment B2G is not a high priority for us particularly given the next release isn't leaving m-c until november.
Flags: needinfo?(bugmail.mozilla)
I don't have a Z3 compact device, so I can't really test - in general we seem to have some problems with hit testing and targeting trying to be too smart and in too many cases you end up tapping the wrong element..
Flags: needinfo?(hsteen)
I nominate this for tracking because it's an incompatibility that will harm us more as we ship Fx OS devices with high resolutions.
tracking-b2g: --- → backlog
[Blocking Requested - why for this release]: This is still happening. France 24 webapp on the Marketplace is nearly unusable. Can we do something?
blocking-b2g: --- → 2.5?
Milan, 

Can you please take a look at this or find a right owner?

Thanks
blocking-b2g: 2.5? → 2.5+
Flags: needinfo?(milan)
Priority: -- → P2
(In reply to Alexandre LISSY :gerard-majax from comment #25)
> [Blocking Requested - why for this release]: This is still happening. France
> 24 webapp on the Marketplace is nearly unusable. Can we do something?

Can you share what is unusable for you? Any good STR? It works fine for me...
Flags: needinfo?(lissyx+mozillians)
(In reply to Julien Wajsberg [:julienw] from comment #27)
> (In reply to Alexandre LISSY :gerard-majax from comment #25)
> > [Blocking Requested - why for this release]: This is still happening. France
> > 24 webapp on the Marketplace is nearly unusable. Can we do something?
> 
> Can you share what is unusable for you? Any good STR? It works fine for me...

Try to view a video. Try to search a station on voyages-sncf as documented in comment 0
Flags: needinfo?(lissyx+mozillians)
(In reply to Alexandre LISSY :gerard-majax from comment #28)
> Try to view a video. Try to search a station on voyages-sncf as documented
> in comment 0

... or follow any link in a mail with fastmail.com. I can try for hours, the only way I have managed to follow the links is to long-tap. It's pretty bad.
(In reply to Alexandre LISSY :gerard-majax from comment #28)
> (In reply to Julien Wajsberg [:julienw] from comment #27)
> > (In reply to Alexandre LISSY :gerard-majax from comment #25)
> > > [Blocking Requested - why for this release]: This is still happening. France
> > > 24 webapp on the Marketplace is nearly unusable. Can we do something?
> > 
> > Can you share what is unusable for you? Any good STR? It works fine for me...
> 
> Try to view a video. Try to search a station on voyages-sncf as documented
> in comment 0

OK so the STR on France 24 is to open a video. Thanks for the detailed STR :p
I debugged the France 24 app and I think the issue is from their side: we properly get the click event, but they try to load a tracking image which fails. Maybe because it's a cross-domain request, not sure.

So the callback to run the video is never called because they don't handle the "error" event.
Spent some time investigating the issue with fastmail, this is different too but likely a blocker so Nical will file a separate bug for this.
(In reply to Julien Wajsberg [:julienw] from comment #33)
> Spent some time investigating the issue with fastmail, this is different too
> but likely a blocker so Nical will file a separate bug for this.

Bug 1216937
I tried to debug voyages-sncf.mobi earlier today but it seems the search functionality is now broken before I see the result page :/
Milan, Is there an owner you can find for this?

Thanks
I don't know if we or they did something, but voyages-sncf.mobi works a lot better for me today.

Alexandre, what do you think ?
Flags: needinfo?(lissyx+mozillians)
No change at all for me. My gecko and Gaia are from the 22th so if we fixed anything I don't have it yet.
Flags: needinfo?(lissyx+mozillians)
This is not cheap to deal with (see comment 7), and is more of a feature request than a bug, when it comes to size.  It would first need to be a P1, rather than P2, and then I'd have to get it escallated to delay some other work in order to get to this.  :mahe - is it worth it?
Flags: needinfo?(milan) → needinfo?(mpotharaju)
(In reply to Milan Sreckovic [:milan] from comment #39)
> This is not cheap to deal with (see comment 7), and is more of a feature
> request than a bug, when it comes to size.  It would first need to be a P1,
> rather than P2, and then I'd have to get it escallated to delay some other
> work in order to get to this.  :mahe - is it worth it?

Technically, the result of that bug is that I am unable to make use of some website on B2G while the same website works on Nightly. And that is with the same device.
(In reply to Alexandre LISSY :gerard-majax from comment #38)
> No change at all for me. My gecko and Gaia are from the 22th so if we fixed
> anything I don't have it yet.

Same with a build from today. Julien told me he might have mistaken that and thought it was okay because of testing on Flame.
Let's keep it as a P2 and have a fix land in few days. Don't want to rush a complicated issue and have you fix it in the next few days. Thanks
Flags: needinfo?(mpotharaju)
Had some preliminary conversations, and the right path seems to be the one mentioned in comment 8; the spec is vague on the subject, so us doing what Chrome does should be a safe bet.  We should be able to get this done and landed, baring major surprises, before 11/13.
Assignee: nobody → bugmail.mozilla
I have patches that fix this problem. Still doing some testing, but there's a try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=85ed4981d855 with the patches if anybody wants to apply them locally and provide feedback. git branch with the patches is at https://github.com/staktrace/gecko-dev/commits/nopanning
Thanks kats! I'm heading to droidcon right now but I'll give a try to this later today :-)
Updated try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=54b834544621 - fixes Android build failure and a failing test. github branch is updated too.
Attached file cherry-pick — 
Cherry-picked, tested, works well for me :)
Thanks for testing!
So after some discussion and thinking I think the approach that I had to fix this needs to be reworked. The good news is that the new approach (bug 1141127) will fix more cases than just this one bug. The bad news is it will take more time to implement, although hopefully I should have something working in the next few days even if it's not ideal.
Depends on: 1141127
No longer depends on: 1219898
See Also: 1141127 → 
This should be fixed by bug 1141127. I'll wait a few days before requesting uplift to b2g44 to reduce chances of fallout.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #50)
> This should be fixed by bug 1141127. I'll wait a few days before requesting
> uplift to b2g44 to reduce chances of fallout.

It is not fixed. I just tested current master on Z3c, verified it properly includes bug 1141127 and I do still have the same problem.
Status: RESOLVED → VERIFIED
Flags: needinfo?(bugmail.mozilla)
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Video please. I couldn't repro the problem when i tried. Also in comment 2 you mentioned a UA override, is that a necessary step to repro?
Flags: needinfo?(bugmail.mozilla) → needinfo?(lissyx+mozillians)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #52)
> Video please. I couldn't repro the problem when i tried. Also in comment 2
> you mentioned a UA override, is that a necessary step to repro?

Yes, if you don't change the UA you don't get the proper website. I don't have the tools to make a video.
Flags: needinfo?(lissyx+mozillians)
Flags: needinfo?(bugmail.mozilla)
I get a perfectly usable website if I go to the URL without an override. Are you expecting the average user to enter an override?
Flags: needinfo?(bugmail.mozilla) → needinfo?(lissyx+mozillians)
Looks like the UA override is not needed anymore.
Flags: needinfo?(lissyx+mozillians)
Are you able to reproduce the problem with clicking on the link on the reduced test case attached to this bug?
I don't reproduce with the attached test case.
Flags: needinfo?(bugmail.mozilla)
Alex, I can help you recording a video on monday.
I tried again on voyages-sncf.mobi with the latest B2G master on Aries and wasn't able to reproduce the issue. I was able to pick cities like "Paris" and "Lyon" in the search field without any problem. There's not much else I can do here without STR that work for me.
Assignee: bugmail.mozilla → nobody
Flags: needinfo?(bugmail.mozilla)
So after some IRC discussion it turned out the difference was in our STR. I was typing "Par" and picking Paris from the list, Alex was typing "Paris". The difference is that when you type "Paris" the list of matching suggestions is much shorter. This means the dropdown box with the suggestions is not scrollable, and so we don't consume the touchmove events, we send them through. Based on my discussion with Rick I believe Chrome should be doing the same thing, although it's possible that on Android the OS is sending touchmove events less frequently or with a greater threshold so that's why the problem doesn't manifest there (or on Firefox for Android, for that matter).

I think a fix here would be to consume touchmove events even if the target APZC is not scrollable, but use a smaller threshold than if it is scrollable. That way we emulate what (I assume) Android is doing, and don't degrade touch latency to games too badly.
Comment on attachment 8684987 [details]
MozReview Request: Bug 1174532 - Even if the APZC is not pannable, have a small slop area to consume touchmove events. r?botond

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24689/diff/1-2/
Attachment #8684987 - Flags: review?(botond)
Assignee: nobody → bugmail.mozilla
Attachment #8684987 - Flags: review?(botond)
Comment on attachment 8684987 [details]
MozReview Request: Bug 1174532 - Even if the APZC is not pannable, have a small slop area to consume touchmove events. r?botond

https://reviewboard.mozilla.org/r/24689/#review22249

::: gfx/layers/apz/src/InputBlockState.cpp:854
(Diff revision 1)
> +      threshold = threshold / 3.0;

Would it make sense to make this a pref so it can be tuned per-device?
Yeah, makes sense. I should have done that, I'm getting really lazy these days for some reason :(
Attachment #8684987 - Attachment description: MozReview Request: Bug 1174532 - Even if the APZC is not pannable, have a small slop area to consume touchmove events. r= → MozReview Request: Bug 1174532 - Even if the APZC is not pannable, have a small slop area to consume touchmove events. r?botond
Attachment #8684987 - Flags: review?(botond)
Comment on attachment 8684987 [details]
MozReview Request: Bug 1174532 - Even if the APZC is not pannable, have a small slop area to consume touchmove events. r?botond

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24689/diff/1-2/
Attachment #8684987 - Flags: review?(botond) → review+
Comment on attachment 8684987 [details]
MozReview Request: Bug 1174532 - Even if the APZC is not pannable, have a small slop area to consume touchmove events. r?botond

https://reviewboard.mozilla.org/r/24689/#review22259
https://hg.mozilla.org/mozilla-central/rev/86ffe5dc026c
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Comment on attachment 8684987 [details]
MozReview Request: Bug 1174532 - Even if the APZC is not pannable, have a small slop area to consume touchmove events. r?botond

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): support for high-density display devices like Aries which send touchmove events with very small finger movements
User impact if declined: on certain web content attempting to tap on stuff fails because we send a touchmove event to the content between the touchdown and touchup
Testing completed: locally, on m-c
Risk to taking this patch (and alternatives if risky): low-medium risk but we don't have much in the way of alternatives. this is the right solution for the long-term but we need to be diligent about catching regressions and fixing them.
String or UUID changes made by this patch: none
Attachment #8684987 - Flags: approval‑mozilla‑b2g44?
Comment on attachment 8684987 [details]
MozReview Request: Bug 1174532 - Even if the APZC is not pannable, have a small slop area to consume touchmove events. r?botond

Approved for 2.5 landing
Attachment #8684987 - Flags: approval‑mozilla‑b2g44? → approval‑mozilla‑b2g44+
This requires bug 1141127 to be uplifted first.
Whiteboard: [NO_UPLIFT]
Adding Aries to the title since it doesn't occur on Flame. I'll be verifying this issue in the next comment.
Summary: Very hard to tap elements in search result on Voyages-SNCF.mobi → [Aries] Very hard to tap elements in search result on Voyages-SNCF.mobi
This issue is verified fixed on Aries central and 2.5.

In addition to Paris, I also tested with a number of different cities in France listed in https://simple.wikipedia.org/wiki/List_of_cities_in_France
I was able to select a city populated in the list, whether I'd finished typing the city name or not.

I did notice another issue which is the X button on right of search bar (tapping on it to clear search) seems a bit difficult to trigger. I'll look into bugging this separately.

Verified on:
Device: Aries 2.6 Master
BuildID: 20151123143009
Gaia: bae13c9ac6a91beecd7c94384e2aef25ed1a3214
Gecko: d3d286102ba7f8801e9dfe12d534f49554ba50c0
Gonk: a19052e4389c3ae2d8fc3e7a74a475401baacc56
Version: 45.0a1 (2.6) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

Device: Aries 2.5
BuildID: 20151123164502
Gaia: 5839f17dedc757947c9531dc0d66c3c49119f5ea
Gecko: 3a45ca93dd447e046baa2c7590f60ae008e438f8
Gonk: a19052e4389c3ae2d8fc3e7a74a475401baacc56
Version: 44.0a2 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-v2.5: fixed → verified
status-b2g-master: fixed → verified
Flags: needinfo?(jmercado)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: