Closed
Bug 193568
Opened 22 years ago
Closed 22 years ago
Can't drag the selected text that is not anchor
Categories
(SeaMonkey :: General, defect)
SeaMonkey
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: masayuki, Assigned: john)
References
Details
(Keywords: regression, Whiteboard: fixed1.3)
Attachments
(1 file, 5 obsolete files)
(deleted),
patch
|
bryner
:
review+
john
:
superreview+
asa
:
approval1.3+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•22 years ago
|
Summary: Can't drag the text that is not anchor → Can't drag the selected text that is not anchor
Reporter | ||
Comment 1•22 years ago
|
||
This bug is regression of Bug 103055 ?
Reporter | ||
Updated•22 years ago
|
Severity: normal → major
Reporter | ||
Comment 2•22 years ago
|
||
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?
Flags: blocking1.3?
Comment 3•22 years ago
|
||
To jkeiser
Reporter | ||
Comment 4•22 years ago
|
||
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.
Assignee | ||
Comment 5•22 years ago
|
||
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.
Status: NEW → ASSIGNED
Comment 6•22 years ago
|
||
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?
Flags: blocking1.3? → blocking1.3+
Comment 7•22 years ago
|
||
text dragging is Broken in Mozilla for BeOS as well
Assignee | ||
Comment 8•22 years ago
|
||
If I don't come up with a fix very soon, we will back out the offending portion
of bug 103055.
Assignee | ||
Comment 9•22 years ago
|
||
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.
Assignee | ||
Comment 10•22 years ago
|
||
OK, I have a patch for this coming up that really makes drag and drop more
reponsive and fixes this bug to boot.
Assignee | ||
Comment 11•22 years ago
|
||
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.
Assignee | ||
Comment 12•22 years ago
|
||
Oops, I didn't attach the one with if (BuildDragData). This is the real one.
Attachment #114963 -
Attachment is obsolete: true
Updated•22 years ago
|
OS: Windows XP → All
Hardware: PC → All
Assignee | ||
Comment 13•22 years ago
|
||
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.
Attachment #114964 -
Attachment is obsolete: true
Assignee | ||
Comment 14•22 years ago
|
||
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.
Assignee | ||
Updated•22 years ago
|
Attachment #114992 -
Flags: review?(bryner)
Comment 15•22 years ago
|
||
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.
Attachment #114992 -
Flags: review?(bryner) → review+
Assignee | ||
Updated•22 years ago
|
Attachment #114992 -
Flags: superreview?(kin)
Comment 16•22 years ago
|
||
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.
Attachment #114992 -
Flags: superreview?(kin) → superreview+
Assignee | ||
Updated•22 years ago
|
Attachment #114992 -
Flags: approval1.3?
Comment 17•22 years ago
|
||
Comment on attachment 114992 [details] [diff] [review]
Patch v2.0
a=asa (on behalf of drivers) for checkin to 1.3
Attachment #114992 -
Flags: approval1.3? → approval1.3+
Assignee | ||
Comment 18•22 years ago
|
||
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.
Assignee | ||
Comment 19•22 years ago
|
||
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.
Attachment #114992 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #115182 -
Flags: review?(bryner)
Assignee | ||
Comment 20•22 years ago
|
||
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.)
Attachment #115182 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #115202 -
Flags: review?(bryner)
Assignee | ||
Updated•22 years ago
|
Attachment #115182 -
Flags: review?(bryner)
Comment 21•22 years ago
|
||
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
Attachment #115202 -
Flags: superreview+
Assignee | ||
Updated•22 years ago
|
Attachment #115202 -
Flags: review?(bryner)
Assignee | ||
Updated•22 years ago
|
Attachment #115209 -
Flags: review?(bryner)
Assignee | ||
Updated•22 years ago
|
Attachment #115209 -
Flags: superreview+
Comment 23•22 years ago
|
||
Comment on attachment 115209 [details] [diff] [review]
Patch v3.1.1
Looks good.
Attachment #115209 -
Flags: review?(bryner) → review+
Assignee | ||
Updated•22 years ago
|
Attachment #115209 -
Flags: approval1.3?
Comment 24•22 years ago
|
||
Comment on attachment 115209 [details] [diff] [review]
Patch v3.1.1
a=asa (on behalf of drivers) for checkin to 1.3
Attachment #115209 -
Flags: approval1.3? → approval1.3+
Assignee | ||
Comment 25•22 years ago
|
||
This was checked in last night and should be in today's builds.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 26•22 years ago
|
||
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 27•22 years ago
|
||
Yes, it does. I can't check it in until the branch opens, however.
Reporter | ||
Comment 28•22 years ago
|
||
I can't reproduce with 2003022304-trunk/WinXP.
Thank you!
But, another problem has a dragging.
See Bug 194459.
Comment 29•22 years ago
|
||
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?
Assignee | ||
Comment 30•22 years ago
|
||
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.
Assignee | ||
Comment 31•22 years ago
|
||
Note that I *am* going to check in this patch to both trunk and branch because
it makes things better for most cases.
Assignee | ||
Comment 32•22 years ago
|
||
Patch checked in to trunk and 1.3 both. bug 194802 filed for the text drag in
textarea issue.
Status: REOPENED → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → FIXED
Updated•22 years ago
|
Whiteboard: fixed1.3
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•