Closed Bug 743279 Opened 8 years ago Closed 8 years ago

Pages with body-level mouse listeners (such as Wikipedia Mobile) don't use touch radius properly

Categories

(Firefox for Android :: General, defect)

x86
Linux
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 15
Tracking Status
firefox14 --- verified
firefox15 --- verified
blocking-fennec1.0 --- +

People

(Reporter: arky, Assigned: kats)

References

Details

Attachments

(3 files, 1 obsolete file)

The Wikipedia mobile version doesn't work in Firefox mobile nightly. 

The expand section buttons are not responsive and
I'm not seeing this issue... I can expand just fine on the wiki for "Red Panda"
Could you give us more details please?  Such as the web page, your device and which version of fennec you are using?
Testing again, on the LG Revolution, it's sometimes occurs with the Nightly 4/19/2012.  It may give haptic feedback but sometimes it does not expand.
This does the same thing for me on Fennec 14 on my Nexus S that I got from IT. It does give haptic feedback, but I'm clicking on the exact right location. Tapping in the same location 3-4 times finally yields the sections expanding.
I find this behaviour on other mediawiki sites such as wikitravel.

http://wikitravel.org/en/Brussels
I'm able to reproduce this on a Galaxy Nexus using latest m-c code on the mobile wikipedia page for the Mona Lisa. It looks like the button to expand sections has a very small area and most of the time I clicked I just ended up hitting the body. Tapping on the body provides haptic feedback because of mouse listeners so it is confusing. The attached screenshot shows the tap target you have to hit to get the section to expand (the small dotted rectangle on the arrow next to "Subject and title". It's small and off-center making it easy to miss. When I inspect the button in firebug on desktop I can see that it has margins that make it wider, so maybe we should include those in the hit-test area. Also just doing a better job of finding nearby targets would help in this case.
Assignee: nobody → bugmail.mozilla
Status: NEW → ASSIGNED
Turns out that when you have a body with event listeners, isElementClickable returns true for everything. Who woulda thunk it. This means everything is clickable, and so we always pick the element exactly under the finger, and we always provide haptic feedback, which makes for a very confused user.

This patch skips over body and root elements with event listeners in some cases, so that we still do the "getClosest" business if the user clicks on an element that is only clickable because of this body thing. Note that the main loop in getClosest (which iterates over all elements in the touch radius) still allows the body to still be selected if there are no better elements, so this shouldn't break any existing good behaviour.

Also this doesn't change the behaviour where you get haptic feedback by tapping anywhere on the page; I can deal with that with one of the other bugs if UX decides it is needed.
Attachment #624519 - Flags: review?(wjohnston)
Attachment #624517 - Flags: review?(wjohnston) → review+
Whoops, missed a null check I meant to put in.
Attachment #624519 - Attachment is obsolete: true
Attachment #624519 - Flags: review?(wjohnston)
Attachment #624523 - Flags: review?(wjohnston)
Also noming for release blocker. I'll probably be duping some of the other dependencies of bug 753525 to this as well.
blocking-fennec1.0: --- → ?
Summary: Wikipedia Mobile: Expand Sections Buttons doesn't work → Pages with body-level mouse listeners (such as Wikipedia Mobile) don't use touch radius properly
Attachment #624523 - Flags: review?(wjohnston) → review+
blocking-fennec1.0: ? → +
https://hg.mozilla.org/mozilla-central/rev/937c5c75431a
https://hg.mozilla.org/mozilla-central/rev/1d0960e138cf
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Comment on attachment 624517 [details] [diff] [review]
(1/2) Minor cleanups

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:
Attachment #624517 - Flags: approval-mozilla-aurora?
Comment on attachment 624523 [details] [diff] [review]
(2/2) Exclude body-with-listeners from being initial target

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: hard to click on things on some pages
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): mobile only
String or UUID changes made by this patch: none
Attachment #624523 - Flags: approval-mozilla-aurora?
Comment on attachment 624517 [details] [diff] [review]
(1/2) Minor cleanups

mobile-only, approved.
Attachment #624517 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #624523 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed on:
Nightly 15.0a1 (2012-05-24)
Aurora 14.0a2 (2012-05-25)
Beta 14.0b3 build 2

Samsung Galaxy SII (2.3.4)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.