311011 - Dragging text that would normally load a bookmark keyword into the new tab button should work

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect, P4)

Description

[Adam Guthrie]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051003 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051003 Firefox/1.6a1

If you have a bookmark keyword such as "bq" that will load the BMO query.cgi
page and you drag the text "bq" onto the new tab button it should work just like
it does when you type it into the URL bar and hit enter.

Reproducible: Always

Steps to Reproduce:
1. Make a bookmark that has a keyword.
2. Drag the keyword text onto the new tab button.

Actual Results:  
We try to load the URL of the keyword string. 

Expected Results:  
The keyword needs to work as expected by loading the keyword URL.

Another issue with this is when you have a bookmark keyword like "bug" that
points to a URL like "https://bugzilla.mozilla.org/show_bug.cgi?id=%s" and you
drag text like "bug 12345" into the URL bar an alert is given saying the URL is
not valid and can't be loaded. Fixing the core issue here would probably resolve
this issue, too.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

Fixes the new window, new tab, and go button.

Comment 2

[Asaf Romano (gone)]

Gavin, can you rename the url variable to something like dragedData? (and make |url| another variable).

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch v2

Comment 4

[:Gavin Sharp [email: [e-mail]]]

(note that the new window case won't work because of bug 317733)

Comment 5

[Asaf Romano (gone)]

Comment on attachment 204330 [details] [diff] [review]
patch v2

r=mano.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/base/content/browser.js; new revision: 1.534;

Comment 7

[tabmix.onemen]

don't you need to fix transferUtils.retrieveURLFromData in case flavour is "text/x-moz-url"
replace:
return aData.toString().split("\n")[0];
???

Comment 8

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #7)
> don't you need to fix transferUtils.retrieveURLFromData in case flavour is
> "text/x-moz-url"
> replace:
> return aData.toString().split("\n")[0];
> ???

I don't understand what you're proposing... what's the problem with that line?

Comment 9

[tabmix.onemen]

this bug fix not fix the code for drop link on tab.
in tabbrowser.xml "onDrop" call transferUtils.retrieveURLFromData
if i understand your fix , in transferUtils.retrieveURLFromData in case flavour is
"text/x-moz-url"
replace:
return aData.toString().split("\n")[0];
with:
var xferData = aData.toString().split("\n");
return xferData[0] || xferData[1];

and in tabbrowser.xml "onDrop"
// We're adding a new tab.
this.loadOneTab(getShortcutOrURI(url), null, null, null, bgLoad);

and
// Load in an existing tab.
var tab = aEvent.originalTarget;
this.getBrowserForTab(tab).loadURI(getShortcutOrURI(url));

you can add the postData:

+     var postData = {};
+     url = getShortcutOrURI(url, postData);
      if (aEvent.originalTarget.localName != "tab") {
         // We're adding a new tab.
-         this.loadOneTab(getShortcutOrURI(url), null, null, null, bgLoad);
+         this.loadOneTab(url, null, null, postData.value, bgLoad);
      }
      else {
         // Load in an existing tab.
         var tab = aEvent.originalTarget;
-        this.getBrowserForTab(tab).loadURI(getShortcutOrURI(url));
+        this.getBrowserForTab(tab).loadURI(url);

         if (this.mCurrentTab != tab && !bgLoad)
         this.selectedTab = tab;
      }

and maybe consider to replace this.getBrowserForTab(tab).loadURI with this.getBrowserForTab(tab).loadURIWithFlags .... to pass the postData.value ??

Comment 10

[tabmix.onemen]

try to drop about:config on the new tab button

javascript console:
Security Error: Content at https://bugzilla.mozilla.org/show_bug.cgi?id=311011 may not load or link to about:config.
Error: uncaught exception: Drop of about:config denied.

about:config don't pass the dragDropSecurityCheck ???

Comment 11

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #9)
> this bug fix not fix the code for drop link on tab.
> in tabbrowser.xml "onDrop" call transferUtils.retrieveURLFromData

Toolkit tabbrowser code (tabbrowser.xml) shouldn't depend on browser (getShortcutOrURI) code, the fact that it already does is a bug that needs to be fixed. So until either tabbrowser is moved to /browser or some other method is devised to handle drops on tabs better, that bug will be present.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #10)
> about:config don't pass the dragDropSecurityCheck ???

Correct. Content is not able to "link" to chrome, even by means of a drag. See bug 285438.

Comment 13

[tabmix.onemen]

OK, thank you for clarified this for me

Comment 14

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/base/content/browser.js; new revision: 1.479.2.57;