669816 - Text selection triggers onclick event

Status: RESOLVED DUPLICATE of bug 654352

(Firefox for Android Graveyard :: General, defect, P3)

Description

[CoJaBo]

Go to http://ilovethecode.com/Javascript/Javascript-Tutorials/onClick_Javascript.shtml
Touch and hold on some text, which selects one word (it should probably be possible to drag to select more, but that doesn't seem to be implemented yet).
Move the end-selection handle over the "Say Hi" link text.
The onclick handler fires numerous times. It should not.

Comment 1

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

Yep. It will continue to do so until bug 667243 and bug 654352 are fixed.

Comment 4

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

this bug itself will never be fixed directly, so I am duping to bug 654352

Comment 5

[Martijn Wargers (dead)]

I don't understand the utils mouseup calls in the patch from bug 661388. Afaict, they seem to be the cause of these click events. Shouldn't utils mousup calls only be done when selection has been done?

Comment 6

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

(In reply to comment #5)
> I don't understand the utils mouseup calls in the patch from bug 661388.
> Afaict, they seem to be the cause of these click events. Shouldn't utils
> mousup calls only be done when selection has been done?

The mouseup calls are needed to get the caret to move position. We will remove all of these mousedown and mouseup calls when the platform API is implemented.

Comment 7

[Martijn Wargers (dead)]

The caret is already moved on mousedown (you can check it by enabling caret browsing in Firefox, then doing a mousedown).

I'm wondering if a different solution like using the 'mousemove' event in combination with mouse events' rangeParent/offset (mentioned in bug 654352, comment 17) would work. You could then easily do the real selection with the existing dom selection api.

Comment 8

[Martijn Wargers (dead)]

Attachment: patch

This fixes the issue and fixes a bunch of oter bugs that are blocking bug 661388.
This is basically mimicking the selecting by mouse you do on Firefox desktop, so that's why no shift-mousedown is needed every time the selection dragger is moved.

It doesn't work well when the end dragger is moved before the start dragger, but I think that would be relatively easy to fix. There is bug 667066, so I guess something like that should be happening.

Comment 9

[Martijn Wargers (dead)]

The patch seems to fix bug 667067, bug 673037, bug 674659 and bug 674791.

Comment 10

[Aaron Train [:aaronmt]]

Isn't bug 654352 taking care of this? Also this bug is marked as a dupe.

Comment 11

[Martijn Wargers (dead)]

Yes, using the api from 654352 could also be a way to fix this.
My patch makes use of the existing drag-selection-by-mouse feature in Firefox. I think there are some benefits to that.
For instance, you don't the patch in bug 673037, which complicates things. Also scrolling for scrollable elements works, when needed.

Comment 12

[Martijn Wargers (dead)]

Talked to mfinkle on irc, he prefers the patch from bug 654352.

Comment 13

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

Martijn - Are you sure this fixes the iframe issue (bug 673037)? I don't see any attempt in the code to dive into the iframe element.

I would be interested to take this patch, or a modified version, for fx7. I just need to understand more about what this patch is doing.

Comment 14

[Martijn Wargers (dead)]

(In reply to comment #13)
> Martijn - Are you sure this fixes the iframe issue (bug 673037)? I don't see
> any attempt in the code to dive into the iframe element.

Sorry, I thought it did, but it doesn't. I haven't actually checked with the testcase in the bug, but a different iframe where it seemingly worked. So bug 673037 is NOT fixed by this patch.

Comment 15

[Martijn Wargers (dead)]

On 'Browser:SelectionStart', a mousedown event is fired. Also a mousemove event is fired, but that shouldn't be necessary, ideally. But for some reason, otherwise the the mousedown event isn't kept alive after a certain amount of time, see xxx comment in the patch.

On "Browser:SelectionMove", mousemove events are fired. Because the mouse is still in the down state, selection is created, just like mouse-drag-selection on Firefox desktop.

On "Browser:SelectionEnd", the mouseup event is fired. This ends the mouse-drag-selection mimicking.

Note that I'm "0" for sendMouseEventToWindow for the clickCount, because I don't want to generate a click event.

This code:
+          if (this.cache.lastDrag == 'start') {
+            var aN = sel.anchorNode;
+            var aO = sel.anchorOffset;
+            sel.collapseToStart();
+            sel.extend(aN, aO);
+            dump('lastDrag:'+this.cache.lastDrag+' anchorOffset: '+aO);
+            this.cache.lastDrag = 'end';
+          }

This is needed to switch the focus point of the selection to the point of the selection handle, when first the one selection handle is used and then the other selection handle.

Like I mentioned at the end of comment 8, this patch has the issue when the end dragger is moved before the start dragger, then you get selection issues.