Last Comment Bug 675215 - CTRL+dragging tab into the bookmarks should get bookmark's title from page title, not the file name
: CTRL+dragging tab into the bookmarks should get bookmark's title from page ti...
Status: VERIFIED FIXED
[testday-20110930]
: verified-beta
Product: Firefox
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 8
Assigned To: Frank Yan (:fryn)
:
Mentors:
Depends on:
Blocks: 674732
  Show dependency treegraph
 
Reported: 2011-07-29 08:09 PDT by Sid
Modified: 2011-09-30 07:40 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.05 KB, patch)
2011-07-29 14:28 PDT, Frank Yan (:fryn)
dao+bmo: review-
Details | Diff | Splinter Review
patch v2 (1.14 KB, patch)
2011-07-29 16:08 PDT, Frank Yan (:fryn)
dao+bmo: review+
Details | Diff | Splinter Review

Description Sid 2011-07-29 08:09:58 PDT
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:8.0a1) Gecko/20110729 Firefox/8.0a1
Build ID: 20110729030824

Steps to reproduce:

1. Open http://www.mozilla.com/en-US/firefox/all.html in a tab.
2. Press and hold CTRL, then drag this tab anywhere into bookmarks.


Actual results:

A new bookmark is created with a title "all.html" (from webpage file name).


Expected results:

New bookmark's title should be "Firefox web browser…etc" (from webpage title), as it was before new CTRL+drag bookmarking was implemented.
Comment 1 Frank Yan (:fryn) 2011-07-29 14:28:18 PDT
Created attachment 549470 [details] [diff] [review]
patch
Comment 2 Dão Gottwald [:dao] 2011-07-29 14:49:21 PDT
Comment on attachment 549470 [details] [diff] [review]
patch

Better to use browser.contentTitle, since tab.label can contain other cruft.
Also, can you do whatever it takes to fix creating a link on the desktop or in a file manager? I guess the text/html flavor does this. See also gIdentityHandler.onDragStart.
Comment 3 Frank Yan (:fryn) 2011-07-29 16:08:14 PDT
Created attachment 549501 [details] [diff] [review]
patch v2

(In reply to comment #2)
> Also, can you do whatever it takes to fix creating a link on the desktop or
> in a file manager? I guess the text/html flavor does this.

No, text/x-moz-url does this too. I tested dragging to the desktop and Windows Explorer on Windows and to the desktop and Finder on OS X.
Pinning a titled shortcut to the start menu on Windows 7 only works in IE9 though, because they use their proprietary .website-extension-associated type. I'm not sure what the mime type for that is.
Comment 4 Frank Yan (:fryn) 2011-07-29 16:11:36 PDT
Is there a reason why one would use this.tabbrowser.getBrowserForTab(tab) instead of tab.linkedBrowser? AFAICT, the former simply returns the latter.
Comment 5 Frank Yan (:fryn) 2011-07-29 17:01:22 PDT
Oh, we currently can't handle those IE9 shortcuts anyway, so until we fix bug 605222 and bug 624070, it doesn't make sense to include that type.
Comment 6 Frank Yan (:fryn) 2011-08-01 18:01:29 PDT
Pushed to fx-team. Will flip status when merged to m-c.
https://hg.mozilla.org/integration/fx-team/rev/d5c5b7841f21
Comment 7 Tim Taubert [:ttaubert] 2011-08-02 05:26:46 PDT
http://hg.mozilla.org/mozilla-central/rev/d5c5b7841f21
Comment 8 Teodosia Pop 2011-09-30 07:40:32 PDT
Verified using Mozilla/5.0 (Windows NT 6.1; rv:8.0) Gecko/20100101 Firefox/8.0

Note You need to log in before you can comment on or make changes to this bug.