193568 - Can't drag the selected text that is not anchor

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3b) Gecko/20030216
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3b) Gecko/20030216

I can't drag the text in any pages.

Reproducible: Always

Steps to Reproduce:
1. Select this text.  www.mozilla.org
2. Drag the selected text
3.

Actual Results:  
I can't drag the selected text and drop to tabber or other application.

Expected Results:  
The selected text is able to drag and drop to tabber or other application.

I could drag with 2003021408-trunk/WinXP.
But, I can't drag with 2003021504 and 2003021604/WinXP and Win2000.

But, If I selected the text between some elements, I can drag the text when mouse
downed at center of the selected text.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

This bug is regression of Bug 103055 ?

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

This problem isn't convenience for BBS of Japanese web site.
When we write url other site, the url is not anchor text.
Because many people don't like the other web site master to get HTTP_REFERER.

-> blocking1.3?

Comment 3

[Boris Zbarsky [:bzbarsky]]

To jkeiser

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Test cases, here.
http://bugzilla.mozilla.gr.jp/attachment.cgi?id=1503&action=view

I can drag with Case2 and 4. but this is not always.

1. Must select the center span element.
2. Must select the left and right side text of span element.
3. Must down button of the mouse on the center span element.

Comment 5

[John Keiser (jkeiser)]

Backing out the portion of the bug 103055 patch that retargets mouse events at
the nearest element parent fixes this.  Not that that's the best end solution
necessarily, mind you.

Comment 6

[Asa Dotzler [:asa]]

John, this is a major loss of functionality. We need this fixed for 1.3. What's
the right thing to do for 1.3? Backout offending portion of 103055 or come up
with a better fix?

Comment 7

[Travis Chase]

text dragging is Broken in Mozilla for BeOS as well

Comment 8

[John Keiser (jkeiser)]

If I don't come up with a fix very soon, we will back out the offending portion
of bug 103055.

Comment 9

[John Keiser (jkeiser)]

The problem here is that nsContentAreaDragDrop::BuildDragData is not starting
the drag, because the target node (the parent of the text) is not in the
selection.  This is a legitimate bug with the way this stuff works: if you set
the mouse near the edge of the text, and drag, it will perform selection instead
of dragging, because the mouse gesture event does not happen until you have left
the selection.

The fix here is to make sure we start the drag on the first mouse gesture.

Comment 10

[John Keiser (jkeiser)]

OK, I have a patch for this coming up that really makes drag and drop more
reponsive and fixes this bug to boot.

Comment 11

[John Keiser (jkeiser)]

Attachment: Patch

This patch makes us create the drag (almost) no matter what, as long as we have
not already started a drag.  Before we were only doing that (inexplicably) if
there was special content in the selection.

The assumption, however, that the "node being dragged" is the node under the
cursor, is still a wrong one and may even lead to wrong selections.  We really
need to get some node out of the selection itself in order to deal with this.

Comment 12

[John Keiser (jkeiser)]

Attachment: Patch v1.0.1

Oops, I didn't attach the one with if (BuildDragData).	This is the real one.

Comment 13

[John Keiser (jkeiser)]

Comment on attachment 114964 [details] [diff] [review]
Patch v1.0.1

This patch is bad for several reasons.	We just need to store whether the mouse
was in the selection when we moused down.  Post-1.3 we should also have this
code grab what content the mouse was over when it was moused down as well (this
is stored in ESM) but we'll need to be surgical for now.

Comment 14

[John Keiser (jkeiser)]

Attachment: Patch v2.0

The problem is simply that the node the selection needs to look for is really
the textnode.  This is definitely less than perfect-looking.  We have no really
easy way of getting at that except to look at the frame and grab the content
from it.  IMO OriginalTarget should get set to this but that would require more
restructuring than we should do at the moment.

Comment 15

[Brian Ryner (not reading)]

Comment on attachment 114992 [details] [diff] [review]
Patch v2.0

Yeah, this should be ok.  But you're right, there really should be a way to get
at this information from the event.

Comment 16

[kinmoz]

Comment on attachment 114992 [details] [diff] [review]
Patch v2.0

Looks ok to me. Do we need this same type of fix in the editor's
nsHTMLDataTransfer::CanDrag() ccode? I think it does this same type of check to
ssee if a node is the sselection.

Comment 17

[Asa Dotzler [:asa]]

Comment on attachment 114992 [details] [diff] [review]
Patch v2.0

a=asa (on behalf of drivers) for checkin to 1.3

Comment 18

[John Keiser (jkeiser)]

Yep, we need this fix in the editor too (it is also broken).  I am going to
consolidate this function into one place (nsIDOMNSEvent).  I had a patch tonight
to put this info in originalTarget, but if people grab originalTarget there is
no way to distinguish between XBL anonymous content and a normal textnode
retargeting.  The patch attached to this bug has the same problem, but it
shouldn't be a problem just for checking if the node is in the selection.

Comment 19

[John Keiser (jkeiser)]

Attachment: Patch v3.0

This version of the patch fixes the bug in a similar way, *and* moves this
stuff over to nsIDOMNSEvent so that the other blockers can take advantage of it
(I have a fix for the double-click in composer that depends on this).  This
fixes the drag in browser window problem.

It also makes originalTarget always be set initially when the nsDOMEvent is
created so that the mouse event target will be in it (this is the definition of
originalTarget).  This fixes the drag in composer problem.

Comment 20

[John Keiser (jkeiser)]

Attachment: Patch v3.1

This version of the patch doesn't touch originalTarget at all, which leads to a
much lower chance of further regression.  (i.e. this patch only really affects
the codepaths we are directly trying to fix.)

Comment 21

[Johnny Stenback (:jst)]

Comment on attachment 115202 [details] [diff] [review]
Patch v3.1

- In nsDOMEvent::GetExplicitOriginalTarget()

+  if (mExplicitOriginalTarget) {
+    *aRealEventTarget = mExplicitOriginalTarget;
+    return NS_OK;

Add an NS_ADDREF(*aRealEventTarget) here, or you'll crash and burn... :-)

sr=jst

Comment 22

[John Keiser (jkeiser)]

Attachment: Patch v3.1.1

With addref ;)

Comment 23

[Brian Ryner (not reading)]

Comment on attachment 115209 [details] [diff] [review]
Patch v3.1.1

Looks good.

Comment 24

[Asa Dotzler [:asa]]

Comment on attachment 115209 [details] [diff] [review]
Patch v3.1.1

a=asa (on behalf of drivers) for checkin to 1.3

Comment 25

[John Keiser (jkeiser)]

This was checked in last night and should be in today's builds.

Comment 26

[Daniel Bratell]

If I'm not mistaken it was checked in after branching for 1.3. It needs to be
checked in to the branch too and I can't find such a checkin.

Comment 27

[John Keiser (jkeiser)]

Yes, it does.  I can't check it in until the branch opens, however.

Comment 28

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I can't reproduce with 2003022304-trunk/WinXP.
Thank you!

But, another problem has a dragging.
See Bug 194459.

Comment 29

[kinmoz]

Comment on attachment 115209 [details] [diff] [review]
Patch v3.1.1

I'm guessing dragging in Plaintext widgets such as textfields and textareas are
still broken since nsPlaintextEditor::CanDrag() is still calling
GetOriginalTarget():

http://lxr.mozilla.org/seamonkey/source/editor/libeditor/text/nsPlaintextDataTr
ansfer.cpp#386


jkeiser, have you done checked all places that	called GetOriginalTarget() via
LXR to see there are other modules that need this fix?

Comment 30

[John Keiser (jkeiser)]

Sigh.  The GetOriginalTarget -> GetExplicitOriginalTarget() part of this fix is
wrong.  It will not grab XBL content.  This means smilies and hrs will not be
dragged currently.

We need to either set originalTarget to the real original target (this caused
problems for some odd reason, perhaps because of my implementation of it) or
have another new method.  I would rather give the former approach a try.

Comment 31

[John Keiser (jkeiser)]

Note that I *am* going to check in this patch to both trunk and branch because
it makes things better for most cases.

Comment 32

[John Keiser (jkeiser)]

Patch checked in to trunk and 1.3 both.  bug 194802 filed for the text drag in
textarea issue.