789781 - Switch to new drag and drop api in tabbrowser

Status: RESOLVED FIXED

(SeaMonkey :: Tabbed Browser, defect)

Description

[Ian Neal]

Attachment: Make the switch

We're still using the old drag and drop API in tabbrowser, so this patch fixes that

Comment 1

[[e-mail]]

Comment on attachment 659564 [details] [diff] [review]
Make the switch

When I tried this out it didn't want to show me the indicator for dropping a link between two tabs. (I didn't actually complete the drag to see whether it would create a new tab or not.)

>-  <script type="application/javascript" src="chrome://global/content/nsDragAndDrop.js"/>
Woohoo! [Is there a bug on removing nsDragAndDrop.js?]

>+              dt.mozSetDataAt("application/x-moz-node", target, 0);
Not sure why Firefox's tabbrowser does this, navigatorDD.js for example uses .mozSourceNode instead.

>+              if (URI) {
Why this change?

>+                dt.mozSetDataAt("text/uri-list", spec + "\n" + title, 0);
IIRC URI lists don't include titles.

>+              if (newIndexBetween == tabIndex ||
>+                  newIndexBetween == tabIndex + 1) {
[Does this not fit on one line?]

>+                url = Services.droppedLinkHandler.dropLink(aEvent, {}, true);
[tabbrowser.xml doesn't normally use Services]

>+              else if (draggedTab.sourceDocument &&
>+                       draggedTab.sourceDocument.defaultView.top == content) {
Did you mean aEvent.dataTransfer.mozSourceDocument?

Comment 2

[Ian Neal]

Attachment: Revised switch

Fixed review comments where possible.
I had spotted the missing drop indicator when I first wrote the patch and had worked out why but forgot about it when submitting. Anyway should be fixed now.

As for removal, bug 719188 mentions flagging nsDragAndDrop.js and nsTransferable.js as deprecated but no confirmation of if and when.

Comment 3

[[e-mail]]

Comment on attachment 659603 [details] [diff] [review]
Revised switch

Not tested yet.

>+              dt.mozSetDataAt("application/x-moz-node", target, 0);
[???]

>+            var dt = aEvent.dataTransfer;
>+            var draggedTab = dt.mozSourceNode;
Nit: move these to where you use them.

>+            var within = draggedTab &&
>+                         draggedTab.parentNode == this.tabContainer;
...
>+            var within = draggedTab &&
>+                         draggedTab.parentNode == this.tabContainer;
Oops.

Comment 4

[Ian Neal]

Attachment: Switch with less within [Checked in: Comment 5]

Fixed duplication and missing removal.
var dt lines moved closer to use.

Comment 5

[Ian Neal]

Comment on attachment 660006 [details] [diff] [review]
Switch with less within [Checked in: Comment 5]

http://hg.mozilla.org/comm-central/rev/ab06a84a717a