1191034 - Allow cluster detection when click occurs on the border of a clickable element

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect)

Description

[Dominique Vincent [:domivinc]]

This bug is a follow up of bug 1135369 (comment 23 - [1])

When a click occurs on a clickable element, the current cluster detection process is not looking for elements outside the clickable element. It doesn’t work well when 2 clickable elements are positioned near each other without space between the 2 elements. If the user clicks on the border of one element, the zoomed view should be displayed.
To take into account the fact that the click occurs on a clickable element, the area used to search for cluster elements will be reduced. The heuristic will be: “If the user is 75% or more on the clickable element, don’t add the other elements in the cluster elements list”.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1135369#c23

Comment 1

[Dominique Vincent [:domivinc]]

Attachment: patch-05082015 3-Bug_1191034___Allow_cluster_detection_when_click_occurs_on_the_border_of_a_clickable_element__r_kats.patch

Michael, I made the changes but I think that the situation is getting worse. The zoomed view is displayed very often.
For instance on the twitter page, without this change the zoomed view was already displayed often because there are a lot of small links. Now with this change, it’s really hard to not get the zoomed view. 
The issue with the twitter page is the following: the size of the touch area used by the cluster detection process is already bigger than the height of a text link. Now with this patch, the clicks are always considered on 2 different clickable elements (all the texts are clickable in this page).

This patch is built on top of the patch of bug 1190541. 
You can download the apk using [1].

Making this change, and thinking about the different comments (from you, Anthony, Mark, …), I really think that there are 2 different points of view regarding the zoomed view: some users want to see it more often and other users don’t.
One way to satisfy this 2 different points of view, could be to implement a sensibility factor with different levels: 0 the zoomed view is never displayed, 5 the zoomed view is display very often.
The sensibility factor will just change the size of the touch area used in the cluster detection process.
In your case you will probably want to use a higher value (4) than Anthony (1).


[1] http://rusez.com/apk/bug1191034/fennec-42.0a1.en-US.android-arm.apk

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Dominique Vincent [:domivinc] from comment #1)
> Making this change, and thinking about the different comments (from you,
> Anthony, Mark, …), I really think that there are 2 different points of view
> regarding the zoomed view: some users want to see it more often and other
> users don’t.

I agree that this is possible and we all have different metrics of how ambiguous our clicks feel to us. However, I find Chrome's algorithm works great and I've never heard anyone complain about it so I wonder if there isn't a balance we can hit here.

I'll try to bring in someone else who has more context on the platform side of things to help out here.

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Dominique, I spoke to Kats and Snorp about the approach we've been taking to trigger the zoomed view and Kats, while missing some context, suggested taking a simpler approach, which sounds similar to my thoughts in bug 1135369 comment 23. He said:

I haven't had the chance to do a detailed think-through on all the
options and history here but if it were up to me I would start by
making the zoomed view pop up *only* in case of actual ambiguity, and
ignore all other considerations like text size, width=device-width,
element borders and whatnot. Actual ambiguity is easy to detect; it
happens if there are multiple links with different targets inside the
radius-enlarged touch box. If that basic algorithm runs produces
undesirable behaviour in some situations we should think carefully
about what it is in that situation that needs to be accounted for.
Perhaps you guys already know of situations where this basic algorithm
falls down - if so please let me know.
---

To me, this implies that we shouldn't build this on top of the cluster approach (as I think is done here), but rather put it aside for now.

What do you think? Do you see any downsides to this approach?

Comment 4

[Dominique Vincent [:domivinc]]

Michael, I totally agree with Kats and Snorp that we should avoid to add specific cases only for the zoomed view (for instance the current bug or bug 1191041 taking into account the size of the clickable elements).
All those changes have been requested in order to “play” with the cluster detection: some of the users want to see more often the zoomed view, other users don’t want to see it too often…
As mentioned earlier, if the users think that the current default is not good, the radius-enlarged touch box can be changed (higher values will display the zoomed view more often, lower values will reduce the cases where the zoomed view is displayed).

Regarding the current algorithm used by the zoomed view, I just want to give you some details about the current implementation:

1- in fact the algorithm used by the cluster detection (used to decide or not to display the zoomed view) has not been implemented for the zoomed view. This algorithm was an existing algorithm used to highlight the closest link (method “GetClosest” in file “PositionedEventTargeting.cpp”). This method looks for the closest link. To get the closest link, this method build a list of the links inside the radius-enlarged touch box (the basic algorithm mentioned by kats and Snorp). The best link is the link with the smallest distance from the touch point. This algorithm was already in place prior the zoomed view.

2- the cluster detection is a very small part added at the end of this “GetClosest” method: the number of links found by the basic algorithm is counted and if at the end of the process, this number is higher than 1: it means that the user clicked on a cluster of links.

3- there is only one exception: when the click occurs on a very small clickable element (size of the element or font size lower than “ui.zoomedview.limitReadableSize”). This element is considered as non readable (by human) and in this case the zoomed view is displayed.


When some issues are detected with the zoomed view (I mean cluster detection issues), sometimes the issue is in the method “GetClosest” and is not specific to the zoomed view part. The issue is also visible here: the closest link is not correctly highlighted. The problem is that no body cares about a problem with an invalid highlighted link. The zoomed view problems are more visible because the zoomed view is displayed or not!
For instance bug 1181763 and bug 1190541 were not specific to the zoomed view, those issues were also on the highlighted link. 
Bug 1188185 was specific to the zoomed view because the counter didn’t manage correctly duplicated links. I started to work on the twitter page issue (bug 1191277) and it’s probably another case where duplicated links are counted and should not (2 links inside a top element with a touch listener).


One proposal could be the following:
1- fix bug 1190541 and bug 1191277,
2- keep in mind the ideas from bug 1191041 and bug 1191034 but do not implement them for the moment,
3- decide the correct value for the radius-enlarged touch box (current values: 6mm x 7mm). For expert mobile users, it's probably too high! For the twitter page where all the font sizes are fixed and very small, those 2 values are probably too high!

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Thanks for the overview, Dominique! I spoke with Kats and Snorp – they agree that your approach is the way to go so full steam ahead! :)

Comment 6

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.