Bug 481986: Loading bookmarks into bookmark list is slow, r=gavin
authorMark Finkle <mfinkle@mozilla.com>
Thu, 09 Apr 2009 01:40:05 -0400
changeset 471 89326eb6a9920505214937df37c53a054d1ad2a6
parent 470 278856ffac9b8932c98de6e2103cebbd8c8d48a6
child 472 6c02bb2eaa0a8d594582407bda565457d291ff1e
push id411
push usermfinkle@mozilla.com
push dateThu, 09 Apr 2009 05:40:34 +0000
reviewersgavin
bugs481986
Bug 481986: Loading bookmarks into bookmark list is slow, r=gavin
chrome/content/bindings.xml
chrome/content/browser-ui.js
themes/hildon/browser.css
themes/wince/browser.css
--- a/chrome/content/bindings.xml
+++ b/chrome/content/bindings.xml
@@ -234,20 +234,29 @@
         <![CDATA[
           let itemId = this.getAttribute("itemid");
           if (itemId)
             this.init(itemId);
         ]]>
       </constructor>
 
       <field name="_itemId">null</field>
-      <field name="_uri"/>
+      <field name="_uri">null</field>
       <field name="_control">null</field>
       <field name="_isEditing">false</field>
 
+      <field name="_ioService" readonly="true">
+        Components.classes["@mozilla.org/network/io-service;1"]
+                  .getService(Components.interfaces.nsIIOService);
+      </field>
+      <field name="_faviconService" readonly="true">
+        Components.classes["@mozilla.org/browser/favicon-service;1"]
+                  .getService(Components.interfaces.nsIFaviconService);
+      </field>
+
       <field name="_nameField">
         document.getAnonymousElementByAttribute(this, "anonid", "name");
       </field>
       <field name="_uriField">
         document.getAnonymousElementByAttribute(this, "anonid", "uri");
       </field>
       <field name="_tagsField">
         document.getAnonymousElementByAttribute(this, "anonid", "tags");
@@ -304,49 +313,37 @@
       <method name="init">
         <parameter name="aItemId"/>
         <body>
           <![CDATA[
             this._itemId = aItemId;
             if (!this._itemId)
               return;
 
-            this.name = PlacesUtils.bookmarks.getItemTitle(this._itemId);
-
-            try {
-              this._uri = PlacesUtils.bookmarks.getBookmarkURI(this._itemId);
-            } catch (e) {}
-
-            if (this._uri) {
-              this.spec = this._uri.spec;
-              let fis = Cc["@mozilla.org/browser/favicon-service;1"].getService(Ci.nsIFaviconService);
+            if (this.hasAttribute("uri")) {
+              this._uri = this._ioService.newURI(this.getAttribute("uri"), null, null);
               let icon = document.getAnonymousElementByAttribute(this, "anonid", "favicon");
-              icon.src = fis.getFaviconImageForPage(this._uri).spec;
+              icon.src = this._faviconService.getFaviconImageForPage(this._uri).spec;
             }
           ]]>
         </body>
       </method>
       <method name="startEditing">
         <parameter name="autoSelect"/>
         <body>
           <![CDATA[
-            if (this._itemId == -1)
+            if (!this._itemId)
               return;
 
             this._isEditing = true;
             if (this.control)
               this.control.activeItem = this;
 
             this.updateFields();
 
-            if (this._uri) {
-              var currentTags = PlacesUtils.tagging.getTagsForURI(this._uri, {});
-              this.tags = currentTags.join(", ");
-            }
-
             this._nameField.focus();
             if (autoSelect)
               this._nameField.select();
           ]]>
         </body>
       </method>
       <method name="stopEditing">
         <parameter name="shouldSave"/>
@@ -369,17 +366,17 @@
       </method>
       <method name="save">
         <body>
           <![CDATA[
             // Update the name
             PlacesUtils.bookmarks.setItemTitle(this._itemId, this.name);
 
             // Update the tags
-            if (this._uri) {
+            if (this._uri && this.type != "folder") {
               var currentTags = PlacesUtils.tagging.getTagsForURI(this._uri, {});
               var tags = this.tagsAsArray;
               if (tags.length > 0 || currentTags.length > 0) {
                 var tagsToRemove = [];
                 var tagsToAdd = [];
                 var i;
                 for (i = 0; i < currentTags.length; i++) {
                   if (tags.indexOf(currentTags[i]) == -1)
@@ -393,34 +390,33 @@
                 if (tagsToAdd.length > 0)
                   PlacesUtils.tagging.tagURI(this._uri, tagsToAdd);
                 if (tagsToRemove.length > 0)
                   PlacesUtils.tagging.untagURI(this._uri, tagsToRemove);
               }
 
               // If the URI was updated change it in the bookmark
               if (this._uri.spec != this.spec) {
-                var ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
-                uri = ios.newURI(this.spec, null, null);
+                uri = this._ioService.newURI(this.spec, null, null);
 
                 PlacesUtils.bookmarks.changeBookmarkURI(this._itemId, this._uri);
               }
             }
           ]]>
         </body>
       </method>
       <method name="remove">
         <body>
           <![CDATA[
             PlacesUtils.bookmarks.removeItem(this._itemId);
 
             // If this was the last bookmark (excluding tag-items and livemark
             // children, see getMostRecentBookmarkForURI) for the bookmark's url,
             // remove the url from tag containers as well.
-            if (this._uri) {
+            if (this._uri && this.type != "folder") {
               if (PlacesUtils.getMostRecentBookmarkForURI(this._uri) == -1) {
                 var tags = PlacesUtils.tagging.getTagsForURI(this._uri, {});
                 PlacesUtils.tagging.untagURI(this._uri, tags);
               }
             }
 
             this.stopEditing(false);
 
@@ -463,17 +459,17 @@
       <xul:image anonid="favicon" class="bookmark-item-image"/>
       <xul:grid flex="1">
         <xul:columns>
           <xul:column flex="1"/>
           <xul:column/>
         </xul:columns>
         <xul:rows>
           <xul:row align="center">
-            <xul:textbox anonid="name" readonly="true" xbl:inherits="value=name"/>
+            <xul:textbox anonid="name" readonly="true" xbl:inherits="value=title"/>
             <xul:hbox align="center" anonid="edit-controls" class="bookmark-manage-controls">
               <xul:image anonid="close-button" class="close-button"
                          onmousedown="document.getBindingParent(this).remove()"/>
               <xul:button anonid="folder-button" label="&editBookmarkMove.label;"
                           oncommand="document.getBindingParent(this).move()"/>
               <xul:button anonid="edit-button" label="&editBookmarkEdit.label;"
                           oncommand="document.getBindingParent(this).startEditing()"/>
               <xul:button anonid="done-button" label="&editBookmarkDone.label;" hidden="true"
@@ -510,17 +506,17 @@
       <xul:image anonid="favicon" class="bookmark-folder-image"/>
       <xul:grid flex="1">
         <xul:columns>
           <xul:column flex="1"/>
           <xul:column/>
         </xul:columns>
         <xul:rows>
           <xul:row align="center">
-            <xul:textbox anonid="name" readonly="true" xbl:inherits="value=name"/>
+            <xul:textbox anonid="name" readonly="true" xbl:inherits="value=title"/>
             <xul:hbox align="center" anonid="edit-controls" class="bookmark-manage-controls">
               <xul:image anonid="close-button" class="close-button"
                          onmousedown="document.getBindingParent(this).remove()"/>
               <xul:button anonid="folder-button" label="&editBookmarkMove.label;"
                           oncommand="document.getBindingParent(this).move()"/>
               <xul:button anonid="edit-button" label="&editBookmarkEdit.label;"
                           oncommand="document.getBindingParent(this).startEditing()"/>
               <xul:button anonid="done-button" label="&editBookmarkDone.label;" hidden="true"
@@ -542,37 +538,39 @@
       </method>
     </implementation>
   </binding>
 
   <binding id="place-label" extends="chrome://browser/content/bindings.xml#place-base">
     <content align="center">
       <xul:spacer xbl:inherits="width=indent"/>
       <xul:image anonid="favicon" class="bookmark-folder-image"/>
-      <xul:label anonid="name" crop="end" flex="1"/>
+      <xul:label anonid="name" crop="end" flex="1" xbl:inherits="value=title"/>
     </content>
   </binding>
 
   <binding id="place-list">
     <content orient="vertical" flex="1">
       <xul:vbox anonid="parent-items" class="place-list-parents">
       </xul:vbox>
       <xul:richlistbox anonid="child-items" class="place-list-children" flex="1"/>
     </content>
     <implementation>
       <constructor>
         <![CDATA[
           this._type = this.getAttribute("type");
           this._mode = this.getAttribute("mode");
-
-          let bs = Cc["@mozilla.org/intl/stringbundle;1"].getService(Ci.nsIStringBundleService);
-          this._bundle = bs.createBundle("chrome://browser/locale/browser.properties");
         ]]>
       </constructor>
-      <field name="_bundle">null</field>
+      <field name="_bundle" readonly="true">
+        Components.classes["@mozilla.org/intl/stringbundle;1"]
+                  .getService(Components.interfaces.nsIStringBundleService)
+                  .createBundle("chrome://browser/locale/browser.properties");
+      </field>
+
       <field name="_type"/>
       <field name="_mode"/>
       <field name="_activeItem">null</field>
       <field name="_ignoreEditing">false</field>
       <field name="_manageUI">false</field>
       <field name="_parents">
         document.getAnonymousElementByAttribute(this, "anonid", "parent-items");
       </field>
@@ -672,20 +670,23 @@
             while (parents.firstChild)
               parents.removeChild(parents.firstChild);
 
             const XULNS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
             var self = this;
 
             let folderId = aRootFolder;
             do {
+              let title = PlacesUtils.bookmarks.getItemTitle(folderId);
+
               let parent = document.createElementNS(XULNS, "placelabel");
               parent.setAttribute("class", "bookmark-folder");
               parent.setAttribute("itemid", folderId);
               parent.setAttribute("indent", 0);
+              parent.setAttribute("title", title);
               parents.insertBefore(parent, parents.firstChild);
 
               // XXX Fix me - use <handler>?
               parent.addEventListener("click", function(e) { self.openFolder(e.target.itemId); }, false);
 
               folderId = PlacesUtils.bookmarks.getFolderIdForItem(folderId);
             } while (folderId != PlacesUtils.bookmarks.placesRoot)
 
@@ -693,17 +694,17 @@
             while (children.firstChild)
               children.removeChild(children.firstChild);
 
             children.scrollBoxObject.scrollTo(0, 0);
 
             let childItems = this._getChildren(aRootFolder);
             for (let i=0; i<childItems.length; i++) {
               let node = childItems[i];
-              this.appendItem(node.itemId, node.type);
+              children.appendChild(this.createItem(node));
             }
 
             // Add the "<new folder>" item
             let newFolder = document.createElementNS(XULNS, "button");
             newFolder.setAttribute("class", "bookmark-folder-new");
             newFolder.setAttribute("label", this._bundle.GetStringFromName("editBookmarkAddFolder"));
             children.appendChild(newFolder);
 
@@ -715,38 +716,40 @@
 
       <method name="addFolder">
         <body>
           <![CDATA[
             let parents = this._parents;
             let parent = parents.lastChild;
             if ("itemId" in parent) {
               let children = this._children;
-              let newId = PlacesUtils.bookmarks.createFolder(parent.itemId, this._bundle.GetStringFromName("editBookmarkNewFolder"), PlacesUtils.bookmarks.DEFAULT_INDEX);
-              let child = this.appendItem(newId, Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER, children.lastChild);
+              let title = this._bundle.GetStringFromName("editBookmarkNewFolder");
+              let newId = PlacesUtils.bookmarks.createFolder(parent.itemId, title, PlacesUtils.bookmarks.DEFAULT_INDEX);
+              let child = this.createItem({ itemId: newId, title: title, type: Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER });
+              children.insertBefore(child, children.lastChild);
               child.startEditing(true);
             }
           ]]>
         </body>
       </method>
 
-      <method name="appendItem">
-        <parameter name="aItemId"/>
-        <parameter name="aType"/>
-        <parameter name="aBefore"/>
+      <method name="createItem">
+        <parameter name="aItem"/>
         <body>
           <![CDATA[
             const XULNS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 
             let child = document.createElementNS(XULNS, "placeitem");
-            child.setAttribute("itemid", aItemId);
+            child.setAttribute("itemid", aItem.itemId);
             child.setAttribute("class", "bookmark-item");
-            if (aType == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER)
+            child.setAttribute("title", aItem.title);
+            child.setAttribute("uri", aItem.uri);
+            child.setAttribute("tags", aItem.tags);
+            if (aItem.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER)
               child.setAttribute("type", "folder");
-            this._children.insertBefore(child, aBefore);
 
             // XXX make a <handler> for the mousedown
             var self = this;
             child.addEventListener("mouseup", function(e) { self._fireOpen(e, child); }, false);
 
             return child;
           ]]>
         </body>
@@ -857,43 +860,44 @@
             aRootFolder = aRootFolder || PlacesUtils.bookmarks.bookmarksMenuFolder;
             aLevel = aLevel || 0;
 
             let items = this._items;
             if (!aLevel) {
               while (items.firstChild)
                 items.removeChild(items.firstChild);
 
-              this.appendItem(aRootFolder, Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER, aLevel);
+              let node = { itemId: aRootFolder, title: PlacesUtils.bookmarks.getItemTitle(aRootFolder), type: Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER };
+              items.appendChild(this.createItem(node, aLevel));
               aLevel += 24;
             }
 
             let childItems = this._getChildren(aRootFolder);
             for (let i=0; i<childItems.length; i++) {
               let node = childItems[i];
-              let child = this.appendItem(node.itemId, node.type, aLevel);
+              items.appendChild(this.createItem(node, aLevel););
+
               this.openFolder(node.itemId, aLevel + 24);
             }
           ]]>
         </body>
       </method>
 
-      <method name="appendItem">
-        <parameter name="aItemId"/>
-        <parameter name="aType"/>
+      <method name="createItem">
+        <parameter name="aItem"/>
         <parameter name="aLevel"/>
         <body>
           <![CDATA[
             const XULNS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 
             let child = document.createElementNS(XULNS, "placelabel");
             child.setAttribute("type", "folder");
-            child.setAttribute("itemid", aItemId);
+            child.setAttribute("itemid", aItem.itemId);
+            child.setAttribute("title", aItem.title);
             child.setAttribute("indent", aLevel);
-            this._items.appendChild(child);
 
             // XXX make a <handler> for the mouseup
             var self = this;
             child.addEventListener("mouseup", function(e) { self._fireSelect(e, child); }, false);
 
             return child;
           ]]>
         </body>
--- a/chrome/content/browser-ui.js
+++ b/chrome/content/browser-ui.js
@@ -606,53 +606,60 @@ var BookmarkHelper = {
   _panel: null,
   _editor: null,
 
   edit: function(aURI) {
     let itemId = PlacesUtils.getMostRecentBookmarkForURI(aURI);
     if (itemId == -1)
       return;
 
+    let title = PlacesUtils.bookmarks.getItemTitle(itemId);
+
     let container = document.getElementById("browser-container");
     this._panel = document.getElementById("bookmark-container");
     this._panel.hidden = false;
     this._panel.width = container.boxObject.width;
 
     this._editor = document.getElementById("bookmark-edit");
+    this._editor.setAttribute("title", title);
+    this._editor.setAttribute("uri", aURI.spec);
     this._editor.init(itemId);
     this._editor.startEditing();
 
     window.addEventListener("keypress", this, true);
   },
 
   close: function() {
     window.removeEventListener("keypress", this, true);
     BrowserUI.updateStar();
 
     if (this._editor.isEditing)
       this._editor.stopEditing();
     this._panel.hidden = true;
+
+    this._editor.removeAttribute("title");
+    this._editor.removeAttribute("uri");
   },
 
   handleEvent: function(aEvent) {
     if (aEvent.keyCode == aEvent.DOM_VK_ESCAPE)
       this.close();
   }
 };
 
 var BookmarkList = {
   _panel: null,
   _bookmarks: null,
 
   show: function() {
     let container = document.getElementById("browser-container");
     this._panel = document.getElementById("bookmarklist-container");
-    this._panel.hidden = false;
     this._panel.width = container.boxObject.width;
     this._panel.height = container.boxObject.height;
+    this._panel.hidden = false;
 
     this._bookmarks = document.getElementById("bookmark-items");
     this._bookmarks.manageUI = false;
     this._bookmarks.openFolder();
 
     window.addEventListener("keypress", this, true);
   },
 
--- a/themes/hildon/browser.css
+++ b/themes/hildon/browser.css
@@ -394,18 +394,16 @@ toolbarbutton.panel-button {
 .bookmark-folder, .bookmark-item {
   padding: 5px 2px;
   border-bottom: 1px solid rgb(207,207,207);
 }
 
 .bookmark-folder textbox[readonly="true"], .bookmark-item textbox[readonly="true"] {
   -moz-appearance: none !important;
   background-color: transparent;
-  padding: 0px 9px !important;
-  margin: 0px !important;
   border: none !important;
   -moz-user-select: none !important;
 }
 
 placelabel.bookmark-folder {
   border-bottom: 1px solid rgb(255,255,255);
 }
 
@@ -421,29 +419,39 @@ placelabel.bookmark-folder {
 
 /* folders have a fixed image */
 .bookmark-folder-image {
   list-style-image: url("chrome://browser/skin/images/folder.png");
 }
 
 /* control the manage controls */
 .bookmark-manage-controls, .bookmark-folder-new {
-  visibility: hidden;
+  visibility: collapse;
 }
 
 placelist[ui="manage"] .bookmark-manage-controls,
 placelist[ui="manage"] .bookmark-folder-new,
 placeitem[ui="manage"] .bookmark-manage-controls {
   visibility: visible;
 }
 
 placeitem[ui="manage"] .bookmark-item-image {
   visibility: collapse;
 }
 
+/* be consistent with the size of placeitem */
+placelabel {
+  height: 34px;
+}
+
+/* use a smaller font size for the manage buttons */
+placeitem .button-text {
+  font-size: 12pt !important;
+}
+
 .close-button {
   -moz-appearance: none;
   border: none;
   margin: 0;
   padding: 0;
   list-style-image: url("chrome://browser/skin/images/folder_close.png");
 }
 
--- a/themes/wince/browser.css
+++ b/themes/wince/browser.css
@@ -394,19 +394,18 @@ toolbarbutton.panel-button {
 .bookmark-folder, .bookmark-item {
   padding: 5px 2px;
   border-bottom: 1px solid rgb(207,207,207);
 }
 
 .bookmark-folder textbox[readonly="true"], .bookmark-item textbox[readonly="true"] {
   -moz-appearance: none !important;
   background-color: transparent;
-  padding: 0px 9px !important;
-  margin: 0px !important;
   border: none !important;
+  -moz-user-select: none !important;
 }
 
 placelabel.bookmark-folder {
   border-bottom: 1px solid rgb(255,255,255);
 }
 
 .bookmark-item-image, .bookmark-folder-image {
   width: 24px;
@@ -420,24 +419,39 @@ placelabel.bookmark-folder {
 
 /* folders have a fixed image */
 .bookmark-folder-image {
   list-style-image: url("chrome://browser/skin/images/folder.png");
 }
 
 /* control the manage controls */
 .bookmark-manage-controls, .bookmark-folder-new {
-  visibility: hidden;
+  visibility: collapse;
 }
 
 placelist[ui="manage"] .bookmark-manage-controls,
-placelist[ui="manage"] .bookmark-folder-new {
+placelist[ui="manage"] .bookmark-folder-new,
+placeitem[ui="manage"] .bookmark-manage-controls {
   visibility: visible;
 }
 
+placeitem[ui="manage"] .bookmark-item-image {
+  visibility: collapse;
+}
+
+/* be consistent with the size of placeitem */
+placelabel {
+  height: 34px;
+}
+
+/* use a smaller font size for the manage buttons */
+placeitem .button-text {
+  font-size: 12pt !important;
+}
+
 .close-button {
   -moz-appearance: none;
   border: none;
   margin: 0;
   padding: 0;
   list-style-image: url("chrome://browser/skin/images/folder_close.png");
 }