Text selection triggers onclick event

RESOLVED DUPLICATE of bug 654352

Status

Firefox for Android Graveyard
General
P3
normal
RESOLVED DUPLICATE of bug 654352
7 years ago
7 years ago

People

(Reporter: CoJaBo, Unassigned)

Tracking

Trunk
All
Android

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
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.
Yep. It will continue to do so until bug 667243 and bug 654352 are fixed.
Depends on: 667243, 654352
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Duplicate of this bug: 668228
Duplicate of this bug: 667097
tracking-fennec: --- → ?
tracking-firefox7: --- → +
tracking-fennec: ? → 7+
this bug itself will never be fixed directly, so I am duping to bug 654352
Status: NEW → RESOLVED
Last Resolved: 7 years ago
No longer depends on: 654352, 667243
Resolution: --- → DUPLICATE
Duplicate of bug: 654352
tracking-fennec: 7+ → ---
tracking-firefox7: + → ---
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?
(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.
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.
Created attachment 549113 [details] [diff] [review]
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.
The patch seems to fix bug 667067, bug 673037, bug 674659 and bug 674791.
Isn't bug 654352 taking care of this? Also this bug is marked as a dupe.
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.
Talked to mfinkle on irc, he prefers the patch from bug 654352.
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.
(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.
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.
Duplicate of this bug: 678664
You need to log in before you can comment on or make changes to this bug.