Bug 446171 - Part 1: Get page titles from tab and not depend on history; r?mak draft
authorScott Wu <scottcwwu@gmail.com>
Mon, 21 Mar 2016 14:57:18 +0800
changeset 357714 0967a5d7b3fcb6cfd74c00f6252a6cca1a85b77b
parent 356555 6a441b2b2997cdbd47b5d80283f6f60d5a9e7489
child 357715 92754b332f6ad60e894acc728ad1e6f6d337d550
push id16823
push userbmo:scwwu@mozilla.com
push dateFri, 29 Apr 2016 05:58:49 +0000
reviewersmak
bugs446171
milestone49.0a1
Bug 446171 - Part 1: Get page titles from tab and not depend on history; r?mak MozReview-Commit-ID: EUQv2p9LxtY
browser/base/content/browser-places.js
browser/base/content/test/general/browser_visibleTabs_bookmarkAllPages.js
browser/components/places/content/bookmarkProperties.js
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -545,21 +545,24 @@ var PlacesCommandHook = {
   /**
    * List of nsIURI objects characterizing the tabs currently open in the
    * browser, modulo pinned tabs.  The URIs will be in the order in which their
    * corresponding tabs appeared and duplicates are discarded.
    */
   get uniqueCurrentPages() {
     let uniquePages = {};
     let URIs = [];
-    gBrowser.visibleTabs.forEach(function (tab) {
-      let spec = tab.linkedBrowser.currentURI.spec;
+
+    gBrowser.visibleTabs.forEach(tab => {
+      let browser = tab.linkedBrowser;
+      let uri = browser.currentURI;
+      let spec = uri.spec;
       if (!tab.pinned && !(spec in uniquePages)) {
         uniquePages[spec] = null;
-        URIs.push(tab.linkedBrowser.currentURI);
+        URIs.push({ uri, title: browser.contentTitle });
       }
     });
     return URIs;
   },
 
   /**
    * Adds a folder with bookmarks to all of the currently open tabs in this
    * window.
--- a/browser/base/content/test/general/browser_visibleTabs_bookmarkAllPages.js
+++ b/browser/base/content/test/general/browser_visibleTabs_bookmarkAllPages.js
@@ -15,17 +15,17 @@ function test() {
 
     gBrowser.showOnlyTheseTabs([tabTwo]);
 
     is(gBrowser.visibleTabs.length, 1, "Only one tab is visible");
 
     let uris = PlacesCommandHook.uniqueCurrentPages;
     is(uris.length, 1, "Only one uri is returned");
 
-    is(uris[0].spec, tabTwo.linkedBrowser.currentURI.spec, "It's the correct URI");
+    is(uris[0].uri.spec, tabTwo.linkedBrowser.currentURI.spec, "It's the correct URI");
 
     gBrowser.removeTab(tabOne);
     gBrowser.removeTab(tabTwo);
     Array.forEach(gBrowser.tabs, function(tab) {
       gBrowser.showTab(tab);
     });
 
     finish();
--- a/browser/components/places/content/bookmarkProperties.js
+++ b/browser/components/places/content/bookmarkProperties.js
@@ -552,20 +552,25 @@ var BookmarkPropertiesPanel = {
 
   /**
    * Returns a childItems-transactions array representing the URIList with
    * which the dialog has been opened.
    */
   _getTransactionsForURIList: function BPP__getTransactionsForURIList() {
     var transactions = [];
     for (let uri of this._URIs) {
-      let title = this._getURITitleFromHistory(uri);
-      let createTxn = new PlacesCreateBookmarkTransaction(uri, -1,
-                                                          PlacesUtils.bookmarks.DEFAULT_INDEX,
-                                                          title);
+      // uri should be an object in the form { url, title }. Though add-ons
+      // could still use the legacy form, where it's an nsIURI.
+      let [_uri, _title] = uri instanceof Ci.nsIURI ?
+        [uri, this._getURITitleFromHistory(uri)] : [uri.uri, uri.title];
+
+      let createTxn =
+        new PlacesCreateBookmarkTransaction(_uri, -1,
+                                            PlacesUtils.bookmarks.DEFAULT_INDEX,
+                                            _title);
       transactions.push(createTxn);
     }
     return transactions;
   },
 
   /**
    * Returns a transaction for creating a new folder item representing the
    * various fields and opening arguments of the dialog.