441585 - Hard to click on any links

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Doug Turner (:dougt)]

I find it very hard to click on any links.  

Load digg.com
Try clicking any of the topics.

It usually takes 4-5 clicks to actually load the link, if your are luck.

I am using the tip of my finger.

Comment 1

[Stuart Parmenter]

is this a problem with how far we zoom in, or just a general problem?  can you explain it a bit more?

Comment 2

[Doug Turner (:dougt)]

just general problems.  It is a fairly obvious when load a page, do not zoom in, then try to click on a link.

I think it has something to do with the point of my mousedown being outside of the actual link's targetable area.  This is a bug to investigate what we can do better.

Comment 3

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

I still see this after landing mb-cleanup

Comment 4

[Madhava Enros [:madhava]]

Can we increase the size of the clickable/tappable region around a link?

Comment 5

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

I had some user feedback from FOSDEM '09 that also talked about hard to click links. Also, clicking the wrong link (if links were close together).

One bit of feedback suggested a "fisheye" style zoom for links. Another suggested a transient button next to the link to allow a "cancel" of the click if it was wrong.

Comment 6

[Stuart Parmenter]

I suggest we just make <a> elements have some wider clickable area... should be able to do via css.

Comment 7

[cmtalbert]

I find this every time I use the product.  I always browse with the stylus and count the number of times I have to click a link to see any indication the link was clicked in Fennec.  Currently we have a (very unscientific) average of about 3, which is an improvement over the alphas. BrowserD has us beat at 1.

Here is my test, all done at default zoom levels
Google search
1. Load a google search.
2. Click on the first search result.
Took me about a 30s to complete this task

CNN
1. load cnn.com
2. Pick one of the "latest news" stories that has a link both above and below it.  Click that.
Took me about 45s to complete this task and I ended up with the click registering on the wrong news story.

Travelocity
1. Load travelocity.com
2. Try to search for a flight and use all the widgetry they have in place.
2a. One of the hardest things to do is click on a date in the pop up calendar that comes up.  
Took 3 minutes and I've managed to send clicks through the popups to other parts of the site due to the way the divs appear/disppear and content shifts around when it thinks you're trying to double tap to zoom.  So far, still haven't sent data to search for a flight yet.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

ok, ctalbert challenged me to the travelocity test, and after 8 minutes I got so frustrated I gave up.  I then fired up microb and <1 minute I had entered all my info and had some initial results.

Comment 9

[Ben Combee (:bcombee)]

This had been discussed on IRC.  At the time, one suggestion would be use CSS to make the clickable area of links extend several pixels outside the normal border that would be used on the desktop.

Comment 10

[Madhava Enros [:madhava]]

this applies to input fields as well -- very hard to focus them

Comment 11

[Ben Combee (:bcombee)]

Stuart indicated that you could use CSS selectors in the base HTML style sheet to help here by extending the clickable region up several pixels, and to the sides a smaller number of pixels.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

I don't think there are any ways of doing this with CSS that won't mess with page layouts.

Comment 13

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: bigger radius, drag right away

As far as I can tell, there are two reasons why clicking on links is hard.  The first is that the users "misses" the link, which is what has been being discussed so far on this bug.  This patch doesn't help that.

The second is that the click turns into a drag.  Any move of more that 10 pixels for content or 20 pixels for chrome is interpreted as a pan.  On a 360 dpi screen, that's 1/36th of an inch, which seems pretty tight to me.  The simple solution is to expand the radius, but that would delay panning by.

This patch eliminates expands the radius to 50 pixels (this should probably be made configurable).  To prevent too long of a delay before panning, it starts the pan right away, and sending a click if we don't move by more than the expanded radius of 50 pixels.

Comment 14

[Jaakko Viitanen]

On touch screen devices the tap area should be ~3mm for finger usability.  

The slight "skidding" doesn't matter that much while panning if activating links is easier.

Comment 15

[Ben Combee (:bcombee)]

Comment on attachment 374745 [details] [diff] [review]
bigger radius, drag right away

1) I'd like to see the calculation of pointInDragRadius pulled out to a method instead of repeated in the code.

2) I was concerned about using Math.pow(exp, 2), but when I looked in our JS code, I see that it's special cased so it shouldn't be significantly slower than a straight multiplication.

3) I don't like that you can grab multiple times from mouse move.  I think this could cause a lot of extra code to be called in other handlers. I'd change the InputHandler.grab code to NOP if the handler that's grabbing is already the one that's grabbed to avoid this.

4) You added a tab character near the end inside cancelPending.  This should be removed.

Comment 16

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch v.2

Comment 17

[Ben Combee (:bcombee)]

Comment on attachment 375204 [details] [diff] [review]
patch v.2

Changes look good

Comment 18

[Ben Combee (:bcombee)]

Comment on attachment 375204 [details] [diff] [review]
patch v.2

undoing my r+... I'm seeing grab() calls outside of a mousedown/mouseup sequence that's breaking other things on desktop

Comment 19

[Ben Combee (:bcombee)]

I'm working on a cleaned up version of blassey's patch as a prereq for my other patch.

Comment 20

[Ben Combee (:bcombee)]

Attachment: patch v3 - cleaned up by bcombee, fixed grab on mousemove with no mousedown

This cleans up Brad's patch, moving stuff into the DragData class and cleaning up some code related with dragstart.  It also fixes a problem that was affecting my click-at-end-of-drag patch on desktop where mouse moves were creating extra grab() calls.

Comment 21

[Ben Combee (:bcombee)]

Attachment: Try #4 - same as last change, but with grab(null) handled correctly

Comment 22

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 375671 [details] [diff] [review]
Try #4 - same as last change, but with grab(null) handled correctly

just to confirm, the only difference between this patch and the previous is allowing grab(null) always.  If so, r=me

Comment 23

[Ben Combee (:bcombee)]

Attachment: Try #5 - now with a fix for bad scrollbox exceptions after awesomebar change

Comment 24

[Ben Combee (:bcombee)]

Without the update in Try #5, I'd get a lot of errors like

JavaScript error: , line 0: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIScrollBoxObject.scrollBy]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://browser/content/InputHandler.js :: _dragStop :: line 300"  data: no]

JavaScript error: , line 0: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIScrollBoxObject.scrollBy]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://browser/content/InputHandler.js :: _dragMove :: line 311"  data: no]

after clicking on an entry in the Awesomebar.  That's because the Chrome drag handler grabs a handle to the scrollbox, but that goes away on a click and we still try to do one final drag.

Comment 25

[Ben Combee (:bcombee)]

Attachment: Try #6 - still has try clause, but also fixes root reason for trying to scroll box after the click

Comment 26

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 375885 [details] [diff] [review]
Try #6 - still has try clause, but also fixes root reason for trying to scroll box after the click

Correct me if I'm wrong, but the real issue we're trying to solve here is the awesome bar going away after we select an it. Could this be solved by keeping the awesome bar around and setting display none on it? 
For the more general case, we could prevent this situation from happening by null testing dragData. My concern is that there is a performance impact from the try-catch.

Comment 27

[Ben Combee (:bcombee)]

The second version of the patch fixes the root cause -- keeping the scrollbox reference pointer after the click has been sent, since we really don't need to reposition anything after a click. 

I tried the code without the try/catch, and it worked without throwing exceptions.  I'll update this bug with the code reverted but with that fix.

Comment 28

[Ben Combee (:bcombee)]

Attachment: Try #7 - removed try/catch, verified that setting targetscrollbox to null fixed issue

Comment 29

[Stuart Parmenter]

Comment on attachment 376010 [details] [diff] [review]
Try #7 - removed try/catch, verified that setting targetscrollbox to null fixed issue

This has some bad behavior (i.e. quickly pan up and down, and you'll end up zooming because we're generating double clicks, I guess?).  Trying to figure out what is actually causing it

Comment 30

[Ben Combee (:bcombee)]

Attachment: Try #8 - found problem with grab() call that made it zoom sometimes

Comment 31

[Stuart Parmenter]

Attachment: #9

this is the same as #8 except changing _dragStart to call dragData._setDragStart rather than the other way around.  

you can still triggger unexpected zooms a lot easier with this patch, i think primarily due to the distance required to move to start a grab vs before.  I think 50px might be too high :/

Comment 32

[Brad Lassey [:blassey] (use needinfo?)]

stuart pushed http://hg.mozilla.org/mobile-browser/rev/590c95c4d9c3

Comment 33

[Joel Maher ( :jmaher ) (UTC -8)]

verified with maemo nightly from 20090818