262306 - Selection follows mouse movement in textarea when drag-selecting in border to outside border.

Status: RESOLVED WORKSFORME

(Core :: DOM: Events, defect)

Description

[Martijn Wargers (dead)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a4) Gecko/20040918 Firefox/0.9.1+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a4) Gecko/20040918 Firefox/0.9.1+

See upcoming testcase. That testcase has an exaggerated border, in order to
easily reproduce the bug.

1- Mousedown on the border of this textarea.
2- Drag the mouse outside the border and outside the textarea.
3- Release the mouse-button.
Result: when returning with the mouse inside the textarea, you'll notice that
the selection follows your mouse-movement.

In IE, you can't select text with mouse-dragging on the border of the textarea,
so there it is not an issue.


Reproducible: Always
Steps to Reproduce:

Actual Results:  
Selection is 'sticky', it follows mouse-movement.

Expected Results:  
The selection should not follow mouse-movement.

Comment 1

[Martijn Wargers (dead)]

Attachment: Testcase

Comment 2

[Martijn Wargers (dead)]

Attachment: Testcase2

This bug is also happening when beginning the selection in the padding area of
an textarea and releasing the mouse-button outside the textarea.
Also, when beginning the selection in the padding area of an textare, you can't
drag-select-scroll the textarea. (that's also the case when beginning selection
in the border).
I've noticed this on http://gmail.com , where they've set a 5px padding on
their reply-textarea fields.

Comment 3

[raditzman]

lla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a4) Gecko/20040918
Firefox/0.9.1+ See upcoming testcase. That testcase has an exaggerated border,
in order to easily reproduce the bug.(In reply to comment #1)
> Created an attachment (id=160648) [edit]
> Testcase
> 
Testcase 3

Comment 4

[Vincent Lefevre]

*** Bug 332823 has been marked as a duplicate of this bug. ***

Comment 5

[Martijn Wargers (dead)]

Still happening in current trunk build.

Comment 6

[Uri Bernstein]

Notes to self (or others) for later:

In nsFrame::CaptureMouse(), for any frame inside the textarea, the textarea's view is sent to GrabMouseEvent. But when the frame is nsTextControlFrame itself (which happens when clicking on the border), a containing view is used, and the mouse events are therefore grabbed for it (which is bad).

So the problem could be stated as following:
While a point on the border of a textarea maps (via GetContentOffsetsFromPoint()) to content inside the textarea, its containing view is not the view defined by the textarea, bur instead the containing view.

Unfortunately, my understanding of views is quite poor.

Comment 7

[Uri Bernstein]

Actually, not "its containing view" but "its nearest capturing view", as returned by GetNearestCapturingView().

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

We do still use views for scrollable things, right roc?
Removing those views could perhaps fix this one.

Comment 9

[Uri Bernstein]

Attachment: patch?

Make a nsTextFrame's mouse capturer be its scrollable view, rather than its containing view.

Note the ugly const_cast. Is there a reason why GetScrollableView() can't be const?

Comment 10

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

I'm a bit confused. In the first testcase, the nearest capturing view for the nsTextControlFrame should be the outermost scrolled view (the view for the CanvasFrame). Yet apparently it isn't, can you explain why?

Comment 11

[Uri Bernstein]

(In reply to comment #10)
> I'm a bit confused. In the first testcase, the nearest capturing view for the
> nsTextControlFrame should be the outermost scrolled view (the view for the
> CanvasFrame). Yet apparently it isn't, can you explain why?
> 

I believe it indeed is the outermost view - which is exactly what causes the problem.
Why do you think it isn't?

Comment 12

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

Then why is dragging behaving differently when you click in the border than when you click completely outside the nsTextControlFrame?

Comment 13

[Uri Bernstein]

Because GetContentOffsetsFromPoint (and GetSelectionClosestFrame) don't care about capturing views (or views at all for that matter), and return a textframe inside the control as the target for a point on the control's border.

Comment 14

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

But when I'm dragging and I move the mouse out of the nsTextControlFrame border, GetContentOffsetsFromPoint and GetSelectionClosestFrame should start returning something outside the nsTextControlFrame, right? Yet it seems they don't.

Comment 15

[Uri Bernstein]

This took me a while to figure out, but I did:
The reason GetContentOffsetsFromPoint doesn't return something outside the nsTextControlFrame, is that the event target in this case (and therefore the frame on which GetContentOffsetsFromPoint is called) is the inner nsHTMLScrollFrame.

So somehow (I didn't research this part), we remember that this is a drag operation that started within the text control's nsHTMLScrollFrame, and all mouse events, up to and including the release, are targeted to this frame (or its descendants).

So it seems the analogous solution for mouse capturing would be that nsFrame::HandlePress will call CaptureMouse() on the frame associated with the content/offset returned by GetContentOffsetsFromPoint (i.e., where the selection really starts), rather than on |this|. Would you prefer such a solution?

Comment 16

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

Yes, that sounds much better!

Comment 17

[Uri Bernstein]

Attachment: patch v2

Like this. I tried to affect the current logic as little as possible.

Comment 18

[Uri Bernstein]

Attachment: patch v2 (corrected)

Removed a change that shouldn't have been there.

Comment 19

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

Comment on attachment 333782 [details] [diff] [review]
patch v2 (corrected)

I like this approach. Maybe it's a bit scary --- it might do something unexpected --- but it feels like the right thing to do.

Comment 20

[Uri Bernstein]

Submitted. Let's hope this doesn't break anything.

Comment 21

[Martijn Wargers (dead)]

Works perfect now!
Using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/20080815064214 Minefield/3.1a2pre

Thank you so much for fixing this one, Uri!

Comment 22

[Olli Pettay [:smaug][[e-mail]]]

Backed out in bug 484238. Reopening.

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

Attachment: a patch

This is not perfect. In practice this prevents selection when done on the border.

Comment 24

[Stephen Donner [:stephend] Not actively reading bugmail]

Mass-removing myself from cc; search for 12b9dfe4-ece3-40dc-8d23-60e179f64ac1 or any reasonable part thereof, to mass-delete these notifications (and sorry!)

Comment 25

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:hsinyi, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 26

[Hsin-Yi Tsai (she/her) [:hsinyi]]

This may have been fixed somewhere. It seems to work well for me.