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)

x86
Windows XP
defect
Not set
major

Tracking

()

VERIFIED FIXED

People

(Reporter: norbert.notz, Assigned: roc)

References

Details

(Keywords: regression, testcase)

Attachments

(6 files, 3 obsolete files)

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
OS: Windows 95 → Windows XP
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.)
Attached file Testcase (obsolete) —
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase
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
Attached file Simpler testcase
Simpler testcase, this time no <table> used, but a div with overflow:hidden.
Attachment #161678 - Attachment is obsolete: true
Hmm, the fix for bug 65917, doesn't seem to have caused this bug. I've tested
this in my debug build.
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!
Flags: blocking1.8b?
Flags: blocking1.8b?
Flags: blocking1.8b2?
Flags: blocking1.8b-
What if you have an onlick handler that changes the size of the link text? does
that cause the same problem?
Attached file Testcase3
Well, onclick is not called when you're dragging.
Using an onmousedown I still get the same problem.
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.
Attached patch fix (obsolete) — Splinter Review
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)
Attached file extended testcase
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.
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.
Attachment #174649 - Attachment is patch: false
Attachment #174649 - Attachment mime type: text/plain → text/html
Comment on attachment 174648 [details] [diff] [review]
fix

sr=bzbarsky
Attachment #174648 - Flags: superreview?(bzbarsky) → superreview+
I've tried the patch, and it works fine here. Thanks, Robert!
Attached file Testcase #5
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-
Attached file Testcase #6
Click on one link - drag another. Is this what we want?
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...
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.
Attached patch fix #3Splinter Review
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 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+
checked in
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Verified. works great on all testcases. Thanks, Robert!
Status: RESOLVED → VERIFIED
can this fix have caused the regression seen in:
Bug 288406 crash when dragging bookmark proxy icon (favicon) from Location Bar ?
I would like to suggest that this patch caused bug 288509
Flags: blocking1.8b2?
Depends on: 296491
Depends on: 289667
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: