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

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Mathieu Pellerin]

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

Comment 1

[Daniel Veditz [:dveditz]]

bug 285438 is still open, only partially fixed. Making this depend on it so we
don't forget.

Comment 2

[Rich Walsh]

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.

Comment 3

[Mathieu Pellerin]

I just scream everytime I get this regression, you can't really ship a final
product with a basic drag and drop function broken

Comment 4

[Mats Palmgren (inactive)]

Regression window: 2005-03-29-05 -- 2005-03-30-05.
I'm pretty sure it was caused by bug 255863.

Comment 5

[Mats Palmgren (inactive)]

Attachment: Testcase

Comment 6

[Mats Palmgren (inactive)]

Attachment: 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?

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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;

Comment 8

[Mats Palmgren (inactive)]

Attachment: Patch rev. 2

Added comments and stopping mouse capture as suggested.

Comment 9

[Mats Palmgren (inactive)]

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 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 187458 [details] [diff] [review]
Patch rev. 2

looks good.

The GetFrameFromNode cleanup sounds good too.

Comment 11

[Peter Van der Beken [:peterv]]

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

Comment 12

[Mats Palmgren (inactive)]

Attachment: Patch rev. 3

Use GetCurrentDoc() instead of GetDocument()

Comment 13

[Mats Palmgren (inactive)]

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?

Comment 14

[Mats Palmgren (inactive)]

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.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

widget/src/cocoa/nsChildView.mm included nsIScrollableView.h and has 'view' in
REQUIRES. So that solution is fine.

Comment 16

[Mats Palmgren (inactive)]

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