drag 'n drop of < map > links found in an < img > is broken

RESOLVED FIXED

Status

()

Core
Event Handling
--
major
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: Mathieu Pellerin, Assigned: mats)

Tracking

({regression, testcase})

Trunk
x86
All
regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050402 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050402 Firefox/1.0+

Probably related to Bug 285438, drag and drop area links found in an img tag
stopped to work.

Reproducible: Always

Steps to Reproduce:
1.Go to http://www.licadho.org/
2.Try to drag a link in the red bar (e.g. 'About Us') to the location bar or the
tab bar

Actual Results:  
Does nothing

Expected Results:  
Should load the url in the current tab or new tab
(Reporter)

Updated

13 years ago
Keywords: regression

Updated

13 years ago
Assignee: firefox → jst
Status: UNCONFIRMED → NEW
Ever confirmed: true
bug 285438 is still open, only partially fixed. Making this depend on it so we
don't forget.
Depends on: 285438
Flags: blocking-aviary1.1?

Comment 2

13 years ago
FWIW... this appears to be a cross-platform problem.  The OS/2 version (which
didn't require any platform-specific changes) also exhibits this bug.
(Reporter)

Comment 3

13 years ago
I just scream everytime I get this regression, you can't really ship a final
product with a basic drag and drop function broken
Severity: normal → major
(Reporter)

Updated

13 years ago
Flags: blocking1.8b3?
(Assignee)

Comment 4

13 years ago
Regression window: 2005-03-29-05 -- 2005-03-30-05.
I'm pretty sure it was caused by bug 255863.
Assignee: jst → mats.palmgren
Blocks: 255863
Component: General → Event Handling
Keywords: testcase
OS: Windows XP → All
Product: Firefox → Core
Version: unspecified → Trunk
(Assignee)

Comment 5

13 years ago
Created attachment 187355 [details]
Testcase
(Assignee)

Comment 6

13 years ago
Created attachment 187358 [details] [diff] [review]
Patch rev. 1

The problem is that <area> content does not have a primary frame, so we call
|StopTrackingDragGesture()| and make an early return.

I suppose there was a reason not to keep the original |inDownFrame| around?
I kept a ref to its content instead.

This patch fixes the bug, but it does cause an assertion in |GetFrameFromNode|:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/xpwidgets/nsBaseDragService.cpp&rev=1.35&root=/cvsroot&mark=277,293#277


when called from here:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/xpwidgets/nsBaseDragService.cpp&rev=1.35&root=/cvsroot&mark=197,215#196


I'm not sure how to deal with that - should it not be possible to drag
frame-less content?
Attachment #187358 - Flags: superreview?(roc)
Attachment #187358 - Flags: review?(roc)
Comment on attachment 187358 [details] [diff] [review]
Patch rev. 1

You certainly should be able to drag content without frames. But all this code
is trying to do it turn off mouse capture. You can do that without getting the
frame. You need to get the pres context, and then the view manager, and then
call viewMan->GrabMouseEvents(nsnull).

This code here looks fine, but add comments to make it clear what the
difference is between
   nsCOMPtr<nsIContent> mGestureDownContent;
and
+  nsCOMPtr<nsIContent> mGestureDownFrameOwner;
Attachment #187358 - Flags: superreview?(roc)
Attachment #187358 - Flags: superreview+
Attachment #187358 - Flags: review?(roc)
Attachment #187358 - Flags: review+
(Assignee)

Comment 8

13 years ago
Created attachment 187458 [details] [diff] [review]
Patch rev. 2

Added comments and stopping mouse capture as suggested.
Attachment #187358 - Attachment is obsolete: true
Attachment #187458 - Flags: superreview?(roc)
Attachment #187458 - Flags: review?(roc)
Attachment #187458 - Flags: approval1.8b3?
(Assignee)

Comment 9

13 years ago
nsBaseDragService::GetFrameFromNode() does not seem all that useful to me and
the assertion it has is bogus anyway.  After this bug is fixed there is only one
remaining call to it AFAICT (in Mac specific code):
http://bonsai.mozilla.org/cvsblame.cgi?&file=/mozilla/widget/src/mac/nsDragService.cpp&root=/cvsroot&mark=130,161#129

We could move it there and make it static...
http://lxr.mozilla.org/seamonkey/search?string=GetFrameFromNode
Comment on attachment 187458 [details] [diff] [review]
Patch rev. 2

looks good.

The GetFrameFromNode cleanup sounds good too.
Attachment #187458 - Flags: superreview?(roc)
Attachment #187458 - Flags: superreview+
Attachment #187458 - Flags: review?(roc)
Attachment #187458 - Flags: review+
Comment on attachment 187458 [details] [diff] [review]
Patch rev. 2

>Index: widget/src/xpwidgets/nsBaseDragService.cpp
>===================================================================

>+  nsCOMPtr<nsIContent> contentNode = do_QueryInterface(aDOMNode);
>+  if (contentNode) {
>+    nsIDocument* doc = contentNode->GetDocument();

GetDocument is deprecated, see
http://lxr.mozilla.org/seamonkey/source/content/base/public/nsIContent.h#78
(Assignee)

Comment 12

13 years ago
Created attachment 187522 [details] [diff] [review]
Patch rev. 3

Use GetCurrentDoc() instead of GetDocument()
Attachment #187458 - Attachment is obsolete: true
Attachment #187522 - Flags: superreview?(roc)
Attachment #187522 - Flags: review?(roc)
Attachment #187522 - Flags: approval1.8b3?
(Assignee)

Updated

13 years ago
Attachment #187458 - Flags: approval1.8b3?
Attachment #187522 - Flags: superreview?(roc)
Attachment #187522 - Flags: superreview+
Attachment #187522 - Flags: review?(roc)
Attachment #187522 - Flags: review+

Updated

13 years ago
Attachment #187522 - Flags: approval1.8b3? → approval1.8b3+
(Assignee)

Comment 13

13 years ago
Index: widget/src/xpwidgets/nsBaseDragService.cpp

+#include "nsIViewManager.h"

This broke the build :-(

I attempted to fix it by adding 'view' to REQUIRES in Makefile.in, but that
did not help...

Help?
(Assignee)

Comment 14

13 years ago
OK, it did fix it after all, it just took two cycles to take effect...

--- widget/src/xpwidgets/Makefile.in    9 Dec 2004 19:28:34 -0000       1.67
+++ widget/src/xpwidgets/Makefile.in    28 Jun 2005 22:04:03 -0000      1.68
@@ -58,6 +58,7 @@
                  htmlparser \
                  uconv \
                  unicharutil \
+                 view \
                  $(NULL)

Let me know if that isn't ok to have there.
widget/src/cocoa/nsChildView.mm included nsIScrollableView.h and has 'view' in
REQUIRES. So that solution is fine.
(Assignee)

Comment 16

13 years ago
OK, thanks.

"Patch rev. 3" was checked in to trunk at 2005-06-28 14:40 PDT
and the additional Makefile.in fix checked in at 2005-06-28 15:04 PDT

-> FIXED
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Flags: blocking1.8b3?
Flags: blocking-aviary1.1?
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.