Closed Bug 311011 Opened 19 years ago Closed 19 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
Firefox 2 alpha1

People

(Reporter: ispiked, Assigned: Gavin)

References

Details

(Keywords: fixed1.8.1)

Attachments

(1 file, 1 obsolete file)

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.
Assignee: nobody → gavin.sharp
Keywords: regression
OS: Windows XP → All
Hardware: PC → All
Version: Trunk → 1.5 Branch
Status: NEW → ASSIGNED
Priority: -- → P4
Target Milestone: --- → Firefox1.6-
Version: 1.5 Branch → Trunk
Attached patch patch (obsolete) — Splinter Review
Fixes the new window, new tab, and go button.
Attachment #202866 - Flags: review?(mconnor)
Whiteboard: [patch-r?]
Attachment #202866 - Flags: review?(mconnor) → review?(bugs.mano)
Gavin, can you rename the url variable to something like dragedData? (and make |url| another variable).
Attachment #202866 - Attachment is obsolete: true
Attachment #202866 - Flags: review?(bugs.mano)
Depends on: 317733
Whiteboard: [patch-r?]
Attached patch patch v2Splinter Review
Attachment #204330 - Flags: review?(bugs.mano)
(note that the new window case won't work because of bug 317733)
Whiteboard: [patch-r?]
Comment on attachment 204330 [details] [diff] [review]
patch v2

r=mano.
Attachment #204330 - Flags: review?(bugs.mano) → review+
mozilla/browser/base/content/browser.js; new revision: 1.534;
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Whiteboard: [patch-r?]
don't you need to fix transferUtils.retrieveURLFromData in case flavour is "text/x-moz-url"
replace:
return aData.toString().split("\n")[0];
???
(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?

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 ??
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 ???
(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.
(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.
OK, thank you for clarified this for me
Attachment #204330 - Flags: branch-1.8.1?(mconnor)
Attachment #204330 - Flags: branch-1.8.1?(mconnor) → branch-1.8.1+
mozilla/browser/base/content/browser.js; new revision: 1.479.2.57;
Keywords: fixed1.8.1
Target Milestone: Firefox 2 → Firefox 2 alpha1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: