Closed
Bug 255863
Opened 20 years ago
Closed 19 years ago
unfocused links cannot be drag&dropped quickly
Categories
(Core :: DOM: UI Events & Focus Handling, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: norbert.notz, Assigned: roc)
References
Details
(Keywords: regression, testcase)
Attachments
(6 files, 3 obsolete files)
279 bytes,
text/html
|
Details | |
771 bytes,
text/html
|
Details | |
537 bytes,
text/html
|
Details | |
756 bytes,
text/html
|
Details | |
783 bytes,
text/html
|
Details | |
25.52 KB,
patch
|
MatsPalmgren_bugz
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.8a3) Gecko/20040816 Firefox/0.9.1+ Build Identifier: Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.8a3) Gecko/20040816 Firefox/0.9.1+ On my old, slow PC I easily can reproduce the following bug: --------------------------------------------------------- - place the mouse-pointer over a link - hold down the left mouse button and quickly... - try to move (drag) the link a bit: The mouse-pointer has not drag&drop style. Now (after we have waited a little bit)again move the mouse: Now the mouse-pointer style is updated to drag&drop. --------------------------------------------------------- - place the mouse-pointer over a link - hold down the left mouse button and quickly... - try to move (drag) the link over another tab: It does not work. The mouse-pointer has hand-style. The link only gets focused but cannot be dropped. Keep the left mouse button hold down... - move the mouse out of the tab-bar and again over a tab: Now the link can be dropped there. --------------------------------------------------------- These problems cannot be reproduced if the link is already focused. So this seems to be a "timing problem" if the link is not already focused. Maybe this bug is hard to reproduce with fast PCs. (I cannot reproduce such problems with IE3.) (Please note that I have found this bug with a latest-trunk nightly under Windows 95. I have not tested other builds. I found this problem under Windows 95 but I guess that also other operating systems are affected.) Maybe this bug is related to bug 36867. Please check this. Reproducible: Always Steps to Reproduce:
Summary: unfocused links cannot be drag&dropped fast → unfocused links cannot be drag&dropped quickly
The problems are a bit difficult to reproduce. It looks like you must be fast enough, so that the mouse-pointer already has left the link when it gets focus.
Assignee: firefox → events
Component: General → Event Handling
Product: Firefox → Browser
QA Contact: firefox.general → ian
Version: unspecified → Trunk
(You have set "Trunk". But I guess it is not a trunk-only problem. But I only have tested a latest-trunk nightly.)
Comment 3•20 years ago
|
||
Dragging the first link is hard when it is not focused yet. The second link is not hard to drag. The only difference between the two links is, that the first spans two lines and the second link just one line. I can't see the bug, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a2) Gecko/2004071408 But I can see the bug, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a3) Gecko/2004071508
Updated•20 years ago
|
Comment 4•20 years ago
|
||
another interesting aspect of the bug (and testcase): The link that is difficult to drag is not difficult to drag when you start on the first line. It is only difficult to drag on the second line. The fix for bug 65917 could maybe have caused this bug, but this could be totally wrong: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-07-14+07%3A00%3A00&maxdate=2004-07-15+08%3A00%3A00&cvsroot=%2Fcvsroot
Keywords: regression
Comment 5•20 years ago
|
||
Simpler testcase, this time no <table> used, but a div with overflow:hidden.
Attachment #161678 -
Attachment is obsolete: true
Comment 6•20 years ago
|
||
Hmm, the fix for bug 65917, doesn't seem to have caused this bug. I've tested this in my debug build.
Comment 7•20 years ago
|
||
Ok, it seems that the "there" fix for bug 151375 (https://bugzilla.mozilla.org/attachment.cgi?id=152223&action=view) is the cause of this bug. I've built the Mozilla source from 2004-07-14 and tested: The bug isn't there. Then I've applied the "there" patch and rebuilt: The bug is there.
The problem is that this bug is hard to reproduce on fast PCs. But nevertheless it should be fixed!
Updated•20 years ago
|
Flags: blocking1.8b?
Flags: blocking1.8b?
Flags: blocking1.8b2?
Flags: blocking1.8b-
Assignee | ||
Comment 9•19 years ago
|
||
What if you have an onlick handler that changes the size of the link text? does that cause the same problem?
Comment 10•19 years ago
|
||
Well, onclick is not called when you're dragging. Using an onmousedown I still get the same problem.
Assignee | ||
Comment 11•19 years ago
|
||
What's happening is that clicking on the link is causing a reflow, and during the reflow we measure the desired width of the overflow:hidden block, and that causes the frame for the line you clicked on to be destroyed (temporarily). That causes nsEventStateManager::ClearFrameRefs to stop the drag in progress.
Assignee | ||
Comment 12•19 years ago
|
||
This patch fixes the bug by having the ESM hold a reference to the content where the drag gesture started. When we need a frame for it, we reaquire the primary frame for the content. The original event target may not have been the primary frame but that doesn't matter in this case. We only use the frame to get the right selection controller and some other bookeeping, and it turns out that we don't actually need to dispatch these events to frame event handling.
Assignee: events → roc
Status: NEW → ASSIGNED
Attachment #174648 -
Flags: superreview?(bzbarsky)
Attachment #174648 -
Flags: review?(mats.palmgren)
Assignee | ||
Comment 13•19 years ago
|
||
This testcase shows that with the patch, we can even drag successfully when the mousedown has completely reframed the content. And if the mousedown happens to destroy the frames, that's OK too.
Assignee | ||
Comment 14•19 years ago
|
||
The trickiest part is setting the right coordinates in the DOM event, because we want to pass the coordinates of the original gesture start, but the content may be in a completely different place now and in fact the widget and view the coordinates were relative to before may no longer exist. I try to deal with this by keeping the coordinates in screen coordinates.
Updated•19 years ago
|
Attachment #174649 -
Attachment is patch: false
Attachment #174649 -
Attachment mime type: text/plain → text/html
Comment 15•19 years ago
|
||
Comment on attachment 174648 [details] [diff] [review] fix sr=bzbarsky
Attachment #174648 -
Flags: superreview?(bzbarsky) → superreview+
Comment 16•19 years ago
|
||
I've tried the patch, and it works fine here. Thanks, Robert!
Comment 17•19 years ago
|
||
Comment 18•19 years ago
|
||
Comment on attachment 174648 [details] [diff] [review] fix Apologies for the late review. This patch causes a regression with Testcase #5 "word1" - the mouse pointer does not change until you reach "line 7". I think the problem is here: >+ nsPoint pt = tmpRect.TopLeft(); >+ if (abs(pt.x - mGestureDownPoint.x) > thresholdX || >+ abs(pt.y - mGestureDownPoint.y) > thresholdY) { the left side is in pixels and the right is twips. (using pixelThresholdX/Y instead works for the "word1" case). Testcase #5 "word2" is still not working with that change though, but it didn't before either...
Attachment #174648 -
Flags: review?(mats.palmgren) → review-
Comment 19•19 years ago
|
||
Click on one link - drag another. Is this what we want?
Assignee | ||
Comment 20•19 years ago
|
||
That's caused by the code that dbaron added for synthetic mouse moves: if (IsTrackingDragGesture() && 481 NS_STATIC_CAST(nsMouseEvent*, aEvent)->reason == 482 nsMouseEvent::eSynthesized) { 483 // This is not a real mouse move, it's synthesized. Reset the 484 // starting point to the current point (since things moved), but 485 // otherwise let the user continue the drag. 486 StopTrackingDragGesture(); 487 BeginTrackingDragGesture(aPresContext, 488 NS_STATIC_CAST(nsGUIEvent*, aEvent), 489 aTargetFrame); 490 } The call to BeginTrackingDragGesture sets mGestureDownContent to the content now under the mouse. If we change things to leave mGestureDownContent alone, then we're practically reverting David's code here. CCing David for comments...
Assignee | ||
Comment 21•19 years ago
|
||
Attachment #174648 -
Attachment is obsolete: true
Hrm, it looks like I introduced that in bug 20022 comment 94, so it looks like that code is the fix for bug 233164 -- the intent was to prevent drag initiation from synthesized mouse moves. So probably that code should only be used if we're not already dragging. That said, it looks like IsTrackingDragGesture should be false in that case, since StopTrackingDragGesture is called by GenerateDragGesture in the case where the mouse *has* moved enough. So I'm not really sure what is going on.
It looks (from a 30 second look at the patch) like you're changing this code to convert to screen coordinates for figuring out whether the mouse has moved enough to initiate a drag. If that's the case, then that code shouldn't be needed anymore (given the reasoning in the previous comment). Also, the last two sentences of my previous comment were based on the assumption that there was some bug caused by the existing code, which I guess you probably weren't implying.
Assignee | ||
Comment 24•19 years ago
|
||
Okay, it sounds like removing that code is the right thing to do; it certainly fixes the issue in testcase #6. It also fixes the word2 issue in testcase #5.
Attachment #178852 -
Attachment is obsolete: true
Attachment #178854 -
Flags: superreview+
Attachment #178854 -
Flags: review?(mats.palmgren)
Comment 25•19 years ago
|
||
Comment on attachment 178854 [details] [diff] [review] fix #3 >+ if (abs(pt.x - mGestureDownPoint.x) > pixelThresholdX || >+ abs(pt.y - mGestureDownPoint.y) > pixelThresholdY) { Perhaps use PR_ABS instead? r=mats
Attachment #178854 -
Flags: review?(mats.palmgren) → review+
Assignee | ||
Comment 26•19 years ago
|
||
checked in
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 27•19 years ago
|
||
Verified. works great on all testcases. Thanks, Robert!
Status: RESOLVED → VERIFIED
Comment 28•19 years ago
|
||
can this fix have caused the regression seen in: Bug 288406 crash when dragging bookmark proxy icon (favicon) from Location Bar ?
Comment 29•19 years ago
|
||
I would like to suggest that this patch caused bug 288509
Updated•19 years ago
|
Flags: blocking1.8b2?
Updated•5 years ago
|
Component: Event Handling → User events and focus handling
You need to log in
before you can comment on or make changes to this bug.
Description
•