946731 - [Cards View] It's difficult to tap the cross in the cards view

Status: NEW

(Core :: Layout, defect, P3)

Description

[Julien Wajsberg [:julienw]]

+++ This bug was initially created as a clone of Bug #945727 +++

STR:
* launch an app
* longpress the home button
* try to close the app pressing the cross button

Expected:
* it's easy to tap on it

Actual:
* it's difficult to tap on it.

Discussed this more with Vivien, the root cause is because the card view main container has the event listener.

This bug will be about changing the event fluffing algorithm to take the candidate frame's size into account: when the frame is bigger, it needs less event fluffing magic, and we want to find farther but smaller candidates.

I think it's wise to not block on 1.3 for this.

Comment 1

[Julien Wajsberg [:julienw]]

Attachment: patch v1

Here is a WIP patch. It seems to work better for me in the card view case, and not inducing any regressions.

On the Wikipedia mobile page, the top left icon works also a lot better, but I don't know exactly why ;) But I still can't trigger the "edit" icon on titles (displaying on the right when the title is opened). 

Other questions that I have:
* why are width/height in nsSize signed int instead of unsigned int? Can we have negative width/height?
* what are "appunits"? Are they physical pixels, or CSS pixels, or something else?
* another approach would be to scale up the box for small elements. Would that be better/easier? (not easier for me for sure!)

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8343705 [details] [diff] [review]
patch v1

I like the global idea and I'm curious to see how it may work in practice. I would like to have the opinion of mbrubeck/mfinkle and roc since those may have opinions on it.

The main idea here is to help choosing a target which is close to the finger in the case where clickable target (a) is included into a big container target (b).

With the current code, the distance from (b) will be 0, and the click will always ends up in it. It sounds good in theory but in practice with the event delegation mechanism use on the web it makes it sometimes hard to click small target contained inside those big blocks.

So the question is, does it worth trying to change the weight for small clickables elements contained into a big clickable element for touch inputs (touch events and mouse events issued from a touch). Or is it too hard to achieve correctly and does not worth the risk ?

Comment 3

[Matt Brubeck (:mbrubeck)]

Comment on attachment 8343705 [details] [diff] [review]
patch v1

This is definitely a valid problem, and this seems like a potentially reasonable solution to me.

It might make it harder to click a large target that has small targets nearby.  Is there a reasonable way we could limit this to cases where the small target is contained within the large target?

An appunit is 1/72 of a px.  You should document the units of the new "small surface" threshold and bonus prefs.  For the other "click fluffing" prefs we use mm, which you might want to mimic for consistency.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8343705 [details] [diff] [review]
patch v1

Review of attachment 8343705 [details] [diff] [review]:
-----------------------------------------------------------------

I agree that it sounds reasonable.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8343705 [details] [diff] [review]
patch v1

I agree that this is a real problem. Let's hope those small targets exist for a reason, and being "tappable" is the reason.

Do we have any type of automated tests for different kinds of markup scenarios? It would be nice to know ahead of time how changes like this affect other situations.

Comment 6

[Julien Wajsberg [:julienw]]

Taking it for now, I want to try various approachs. But if this is urgent please steal it since this won't be my main occupation this week.

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: testcase-fluffing-1.html

I made a dumb testcase during the break.

Comment 8

[Julien Wajsberg [:julienw]]

BTW, this is really important and I have a hard time finding time to finish this... so I'll be glad if someone jumps in to finish this.

Comment 9

[Julien Wajsberg [:julienw]]

Attachment: test-fluffing.html

Here is a new testcase.

In Firefox, when enabling the fluffing prefs, we see very easily with the mouse when the "hover" state starts.

Comment 10

[Julien Wajsberg [:julienw]]

I see that bug 1126989 landed a patch that tries to fix this as well, but this testcase is still failing, probably because bug 1126989 focuses on the text size.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

As discussed on IRC, bug 1126989 is orthogonal to this change, as it doesn't touch the heuristics used in picking the click target. It only identifies whether or not there is a cluster of links at the target, and sets a flag based on that.

Comment 12

[Julien Wajsberg [:julienw]]

Attachment: test-fluffing.html

It's better with the right file.

Comment 13

[Julien Wajsberg [:julienw]]

Attachment: rebased patch v1

Comment 14

[Julien Wajsberg [:julienw]]

I don't think I'll ever move forward with this.

Feel free to close if you think it's useless.