Bug 581252 - Replace 'Manage' button in the Bookmarks List with a context menu item [r=mfinkle]
authorVivien Nicolas <21@vingtetun.org>
Fri, 23 Jul 2010 16:49:56 -0400
changeset 66396 b4baf8ea12fad015f403aeb23920fedc8d40e522
parent 66395 baccee8dfec40b448dda454e5add20c1760b1622
child 66397 7686681231cad3f24beeab45d07ffbabf6bb2a29
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmfinkle
bugs581252
Bug 581252 - Replace 'Manage' button in the Bookmarks List with a context menu item [r=mfinkle]
mobile/app/mobile.js
mobile/chrome/content/InputHandler.js
mobile/chrome/content/bindings.xml
mobile/chrome/content/browser-ui.js
mobile/chrome/content/browser.xul
mobile/locales/en-US/chrome/browser.dtd
--- a/mobile/app/mobile.js
+++ b/mobile/app/mobile.js
@@ -54,17 +54,17 @@ pref("zoom.minPercent", 20);
 pref("zoom.maxPercent", 400);
 pref("toolkit.zoomManager.zoomValues", ".2,.3,.5,.67,.8,.9,1,1.1,1.2,1.33,1.5,1.7,2,2.4,3,4");
 pref("zoom.dpiScale", 150);
 
 /* use custom widget for html:select */
 pref("ui.use_native_popup_windows", true);
 
 /* use long press to display a context menu */
-pref("ui.click_hold_context_menus", false);
+pref("ui.click_hold_context_menus", true);
 
 /* cache prefs */
 pref("browser.cache.disk.enable", false);
 pref("browser.cache.disk.capacity", 0); // kilobytes
 
 pref("browser.cache.memory.enable", true);
 pref("browser.cache.memory.capacity", 1024); // kilobytes
 
--- a/mobile/chrome/content/InputHandler.js
+++ b/mobile/chrome/content/InputHandler.js
@@ -121,16 +121,17 @@ function InputHandler(browserViewContain
   /* when set to true, next click won't be dispatched */
   this._suppressNextClick = false;
 
   /* these handle dragging of both chrome elements and content */
   window.addEventListener("mousedown", this, true);
   window.addEventListener("mouseup", this, true);
   window.addEventListener("mousemove", this, true);
   window.addEventListener("click", this, true);
+  window.addEventListener("contextmenu", this, false);
   window.addEventListener("MozMagnifyGestureStart", this, true);
   window.addEventListener("MozMagnifyGestureUpdate", this, true);
   window.addEventListener("MozMagnifyGesture", this, true);
 
   /* these handle key strokes in the browser view (where page content appears) */
   browserViewContainer.addEventListener("keypress", this, false);
   browserViewContainer.addEventListener("keyup", this, false);
   browserViewContainer.addEventListener("keydown", this, false);
@@ -341,29 +342,33 @@ function MouseModule(owner, browserViewC
                                         Util.bind(this._kineticStop, this));
 
   messageManager.addMessageListener("Browser:ContextMenu", this);
 }
 
 
 MouseModule.prototype = {
   handleEvent: function handleEvent(aEvent) {
-    if (aEvent.button !== 0)
+    if (aEvent.button !== 0 && aEvent.type != "contextmenu")
       return;
 
     switch (aEvent.type) {
       case "mousedown":
         this._onMouseDown(aEvent);
         break;
       case "mousemove":
         this._onMouseMove(aEvent);
         break;
       case "mouseup":
         this._onMouseUp(aEvent);
         break;
+      case "contextmenu":
+        if (ContextHelper.popupState && this._dragData.dragging)
+          this._doDragStop(0, 0, true);
+        break;
       case "MozMagnifyGestureStart":
       case "MozMagnifyGesture":
         // disallow kinetic panning after gesture
         if (this._dragData.dragging)
           this._doDragStop(0, 0, true);
         break;
     }
   },
@@ -489,17 +494,17 @@ MouseModule.prototype = {
         this._commitAnotherClick();
       else
         // clean the click buffer ourselves, since there was no clicker
         // to commit to.  when there is one, the path taken through
         // _commitAnotherClick takes care of this.
         this._cleanClickBuffer();
     }
     else if (dragData.isPan()) {
-      // User was panning around, do not allow chrome click
+      // User was panning around or contextmenu was open, do not allow chrome click
       // XXX Instead of having suppressNextClick, we could grab until click is seen
       // and THEN ungrab so that owner does not need to know anything about clicking.
       let generatesClick = aEvent.detail;
       if (generatesClick)
         this._owner.suppressNextClick();
     }
 
     let clicker = this._clicker;
@@ -838,22 +843,18 @@ DragData.prototype = {
     }
   },
 
   setDragPosition: function setDragPosition(sX, sY) {
     // Check if drag is now a pan.
     if (!this._isPan) {
       let distanceSquared = (Math.pow(sX - this._originX, 2) + Math.pow(sY - this._originY, 2));
       this._isPan = (distanceSquared > Math.pow(this._dragRadius, 2));
-      if (this._isPan) {
-        // dismiss the active state of the pan element
-        let target = document.documentElement;
-        let state = this._domUtils.getContentState(target);
-        this._domUtils.setContentState(target, state & kStateActive);
-      }
+      if (this._isPan)
+        this._resetActive();
     }
 
     // If now a pan, mark previous position where panning was.
     if (this._isPan) {
       let absX = Math.abs(this._originX - sX);
       let absY = Math.abs(this._originY - sY);
 
       if (absX > kAxisLockRevertThreshold || absY > kAxisLockRevertThreshold)
@@ -900,16 +901,17 @@ DragData.prototype = {
     this.sX = this._originX = screenX;
     this.sY = this._originY = screenY;
     this.dragging = true;
     this.locked = false;
     this.stayLocked = false;
   },
 
   endDrag: function endDrag() {
+    this._resetActive();
     this.dragging = false;
   },
 
   /** Returns true if drag should pan scrollboxes.*/
   isPan: function isPan() {
     return this._isPan;
   },
 
@@ -921,16 +923,23 @@ DragData.prototype = {
   /**
    * Returns the screen position for a pan. This factors in axis locking.
    * @return Array of screen X and Y coordinates
    */
   panPosition: function panPosition() {
     return this._lockAxis(this.sX, this.sY);
   },
 
+  _resetActive: function _resetActive() {
+    // dismiss the active state of the pan element
+    let target = document.documentElement;
+    let state = this._domUtils.getContentState(target);
+    this._domUtils.setContentState(target, state & kStateActive);
+  },
+
   toString: function toString() {
     return '[DragData] { sX,sY=' + this.sX + ',' + this.sY + ', dragging=' + this.dragging + ' }';
   }
 };
 
 
 /**
  * KineticController - a class to take drag position data and use it
--- a/mobile/chrome/content/bindings.xml
+++ b/mobile/chrome/content/bindings.xml
@@ -373,18 +373,37 @@
     </handlers>
   </binding>
 
   <binding id="place-base">
     <content/>
 
     <handlers>
       <handler event="click" button="0">
-        if (this.control)
+        <![CDATA[
+          let popupState = ContextHelper.popupState;
+          if (!this.control || (popupState && popupState.target == event.originalTarget))
+            return;
+
           this.control._fireOpen(event, this);
+        ]]>
+      </handler>
+      <handler event="contextmenu" phase="capturing">
+        <![CDATA[
+          if (!this.uri)
+            return;
+
+          let data = {
+            target: this,
+            json: {
+              onBookmark: true,
+              bookmarkURL: this.uri.spec
+          }};
+          ContextHelper.showPopup(data);
+        ]]>
       </handler>
     </handlers>
 
     <implementation>
       <field name="_uri">null</field>
       <field name="_control">null</field>
       <field name="_isEditing">false</field>
 
@@ -480,17 +499,17 @@
       <method name="stopEditing">
         <parameter name="shouldSave"/>
         <body>
           <![CDATA[
             if (shouldSave)
               this.save();
 
             this._isEditing = false;
-            if (this.control) {
+            if (this.control && this.control.activeItem) {
               this.control.activeItem.removeAttribute("selected");
               this.control.activeItem = null;
             }
 
             this.updateFields();
 
             let focusedElement = document.commandDispatcher.focusedElement;
             if (focusedElement)
@@ -611,20 +630,18 @@
       <xul:hbox anonid="bookmark-manage" class="bookmark-manage" hidden="true" flex="1">
         <xul:image xbl:inherits="src"/>
         <xul:vbox flex="1">
           <xul:vbox flex="1">
             <xul:textbox anonid="name" xbl:inherits="value=title"/>
             <xul:textbox anonid="uri" xbl:inherits="value=uri"/>
             <xul:textbox anonid="tags" xbl:inherits="value=tags" emptytext="&editBookmarkTags.label;"/>
           </xul:vbox>
-        
+
          <xul:hbox class="bookmark-controls" align="center">
-            <xul:button anonid="remove-button" class="bookmark-remove" label="&editBookmarkRemove.label;"
-                        oncommand="document.getBindingParent(this).remove()"/>
             <xul:spacer flex="1"/>
             <xul:button anonid="done-button" class="bookmark-done" label="&editBookmarkDone.label;"
                         oncommand="document.getBindingParent(this).stopEditing(true)"/>
           </xul:hbox>
         </xul:vbox>
       </xul:hbox>
     </content>
 
@@ -710,17 +727,17 @@
             title: PlacesUtils.bookmarks.getItemTitle(PlacesUtils.bookmarks.unfiledBookmarksFolder),
             type: Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER
           },
           {
             itemId: PlacesUtils.bookmarksMenuFolderId, tags: "", uri: "",
             title: PlacesUtils.bookmarks.getItemTitle(PlacesUtils.bookmarksMenuFolderId),
             type: Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER
           },
-          { 
+          {
             itemId: PlacesUtils.toolbarFolderId, tags: "", uri: "",
             title: PlacesUtils.bookmarks.getItemTitle(PlacesUtils.toolbarFolderId),
             type: Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER
           }
         ];
       ]]></field>
 
       <field name="_readOnlyFolders"><![CDATA[
@@ -731,17 +748,16 @@
           PlacesUtils.toolbarFolderId
         ];
       ]]></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>
       <field name="_children">
         document.getAnonymousElementByAttribute(this, "anonid", "child-items");
       </field>
 
       <field name="scrollBoxObject">this._children.scrollBoxObject</field>
@@ -778,45 +794,16 @@
 
               this._activeItem = val;
             }
             return val;
           ]]>
         </setter>
       </property>
 
-      <property name="manageUI">
-        <getter>
-          <![CDATA[
-            return this._manageUI;
-          ]]>
-        </getter>
-        <setter>
-          <![CDATA[
-            if (this._manageUI != val) {
-              this._manageUI = val;
-              if (this._manageUI) {
-                this.setAttribute("ui", "manage");
-                if (this.getAttribute("autoedit") == "true" && this._children.hasChildNodes()) {
-                  let first = this._getFirstVisibleChild();
-                  if (first)
-                    first.startEditing();
-                }
-              }
-              else {
-                if (this._activeItem && this._activeItem.isEditing)
-                  this._activeItem.stopEditing();
-                this.removeAttribute("ui");
-              }
-            }
-            return val;
-          ]]>
-        </setter>
-      </property>
-
       <method name="isDesktopFolderEmpty">
         <body>
           <![CDATA[
             let options = PlacesUtils.history.getNewQueryOptions();
             options.queryType = options.QUERY_TYPE_BOOKMARKS;
             let query = PlacesUtils.history.getNewQuery();
             let folders = [ PlacesUtils.bookmarks.unfiledBookmarksFolder,
                             PlacesUtils.bookmarksMenuFolderId,
@@ -976,21 +963,16 @@
         <parameter name="aEvent"/>
         <parameter name="aItem"/>
         <body>
           <![CDATA[
             let target = aEvent.originalTarget;
             if (target.localName == "button" || this._activeItem == aItem)
               return;
 
-            if (this._manageUI && this._readOnlyFolders.indexOf(parseInt(aItem.itemId)) == -1) {
-              aItem.startEditing();
-              return;
-            }
-
             if (aItem.type == "folder") {
               this.openFolder(aItem.itemId);
             }
             else {
               // Force the item to be active
               this._activeItem = aItem;
 
               // This is a callback used to forward information to some
--- a/mobile/chrome/content/browser-ui.js
+++ b/mobile/chrome/content/browser-ui.js
@@ -1359,17 +1359,16 @@ var BookmarkHelper = {
     this._panel.hidden = true;
     BrowserUI.popPopup();
   },
 };
 
 var BookmarkList = {
   _panel: null,
   _bookmarks: null,
-  _manageButtton: null,
 
   get mobileRoot() {
     let items = PlacesUtils.annotations.getItemsWithAnnotation("mobile/bookmarksRoot", {});
     if (!items.length)
       throw "Couldn't find mobile bookmarks root!";
 
     delete this.mobileRoot;
     return this.mobileRoot = items[0];
@@ -1378,56 +1377,33 @@ var BookmarkList = {
   show: function() {
     this._panel = document.getElementById("bookmarklist-container");
     this._panel.width = window.innerWidth;
     this._panel.height = window.innerHeight;
     this._panel.hidden = false;
     BrowserUI.pushDialog(this);
 
     this._bookmarks = document.getElementById("bookmark-items");
-    this._bookmarks.addEventListener("BookmarkRemove", this, true);
-    this._bookmarks.manageUI = false;
     this._bookmarks.openFolder();
-
-    this._manageButton = document.getElementById("tool-bookmarks-manage");
-    this._manageButton.disabled = (this._bookmarks.items.length == 0);
   },
 
   close: function() {
     BrowserUI.updateStar();
 
-    if (this._bookmarks.manageUI)
-      this.toggleManage();
     this._bookmarks.blur();
-    this._bookmarks.removeEventListener("BookmarkRemove", this, true);
-
     this._panel.hidden = true;
     BrowserUI.popDialog();
   },
 
-  toggleManage: function() {
-    this._bookmarks.manageUI = !(this._bookmarks.manageUI);
-    this._manageButton.checked = this._bookmarks.manageUI;
-  },
-
   openBookmark: function() {
     let item = this._bookmarks.activeItem;
     if (item.spec) {
       this.close();
       BrowserUI.goToURI(item.spec);
     }
-  },
-
-  handleEvent: function(aEvent) {
-    if (aEvent.type == "BookmarkRemove") {
-      if (this._bookmarks.isRootFolder && this._bookmarks.items.length == 1) {
-        this._manageButton.disabled = true;
-        this.toggleManage();
-      }
-    }
   }
 };
 
 
 var FindHelperUI = {
   type: "find",
   commands: {
     next: "cmd_findNext",
@@ -2037,53 +2013,39 @@ var SelectHelperUI = {
       index: aIndex,
       selected: aSelected,
       clearAll: aClearAll
     };
     Browser.selectedBrowser.messageManager.sendAsyncMessage("FormAssist:ChoiceSelect", json);
   }
 };
 
-const kXLinkNamespace = "http://www.w3.org/1999/xlink";
-
 var ContextHelper = {
   popupState: null,
+  showPopup: function ch_showPopup(aData) {
+    this.receiveMessage(aData);
+  },
 
   receiveMessage: function ch_receiveMessage(aMessage) {
     this.popupState = aMessage.json;
-    this.popupState.browser = aMessage.target;
+    this.popupState.target = aMessage.target;
 
     let first = null;
     let last = null;
     let commands = document.getElementById("context-commands");
     for (let i=0; i<commands.childElementCount; i++) {
       let command = commands.children[i];
       let types = command.getAttribute("type").split(/\s+/);
       command.removeAttribute("selector");
-      if (types.indexOf("image") != -1 && this.popupState.onImage) {
-        first = (first ? first : command);
-        last = command;
-        command.hidden = false;
-        continue;
-      } else if (types.indexOf("image-loaded") != -1 && this.popupState.onLoadedImage) {
-        first = (first ? first : command);
-        last = command;
-        command.hidden = false;
-        continue;
-      } else if (types.indexOf("link") != -1 && this.popupState.onSaveableLink) {
-        first = (first ? first : command);
-        last = command;
-        command.hidden = false;
-        continue;
-      } else if (types.indexOf("callto") != -1 && this.popupState.onVoiceLink) {
-        first = (first ? first : command);
-        last = command;
-        command.hidden = false;
-        continue;
-      } else if (types.indexOf("mailto") != -1 && this.popupState.onLink && this.popupState.linkProtocol == "mailto") {
+      if ((types.indexOf("image") != -1 && this.popupState.onImage) ||
+         (types.indexOf("image-loaded") != -1 && this.popupState.onLoadedImage) ||
+         (types.indexOf("link") != -1 && this.popupState.onSaveableLink) ||
+         (types.indexOf("callto") != -1 && this.popupState.onVoiceLink) ||
+         (types.indexOf("mailto") != -1 && this.popupState.onLink && this.popupState.linkProtocol == "mailto") ||
+         (types.indexOf("edit-bookmark") != -1 && this.popupState.onBookmark)) {
         first = (first ? first : command);
         last = command;
         command.hidden = false;
         continue;
       }
       command.hidden = true;
     }
 
@@ -2095,16 +2057,18 @@ var ContextHelper = {
     first.setAttribute("selector", "first-child");
     last.setAttribute("selector", "last-child");
 
     let label = document.getElementById("context-hint");
     if (this.popupState.onImage)
       label.value = this.popupState.mediaURL;
     if (this.popupState.onLink)
       label.value = this.popupState.linkURL;
+    if (this.popupState.onBookmark)
+      label.value = this.popupState.bookmarkURL;
 
     let container = document.getElementById("context-popup");
     container.hidden = false;
 
     // Make sure the container is at least sized to the content
     let preferredHeight = 0;
     for (let i=0; i<container.childElementCount; i++) {
       preferredHeight += container.children[i].getBoundingClientRect().height;
@@ -2133,18 +2097,28 @@ var ContextHelper = {
 };
 
 var ContextCommands = {
   openInNewTab: function cc_openInNewTab(aEvent) {
     Browser.addTab(ContextHelper.popupState.linkURL, false, Browser.selectedTab);
   },
 
   saveImage: function cc_saveImage(aEvent) {
-    let browser = ContextHelper.popupState.browser;
+    let browser = ContextHelper.popupState.target;
     saveImageURL(ContextHelper.popupState.mediaURL, null, "SaveImageTitle", false, false, browser.documentURI);
+  },
+
+  editBookmark: function cc_editBookmark(aEvent) {
+    let target = ContextHelper.popupState.target;
+    target.startEditing();
+  },
+
+  removeBookmark: function cc_removeBookmark(aEvent) {
+    let target = ContextHelper.popupState.target;
+    target.remove();
   }
 }
 
 function removeBookmarksForURI(aURI) {
   //XXX blargle xpconnect! might not matter, but a method on
   // nsINavBookmarksService that takes an array of items to
   // delete would be faster. better yet, a method that takes a URI!
   let itemIds = PlacesUtils.getBookmarksForURI(aURI);
--- a/mobile/chrome/content/browser.xul
+++ b/mobile/chrome/content/browser.xul
@@ -485,18 +485,16 @@
         </radiogroup>
       </arrowscrollbox>
     </vbox>
 
     <!-- bookmark window -->
     <vbox id="bookmarklist-container" class="panel-dark" hidden="true">
       <hbox id="bookmarklist-header">
         <description flex="1">&bookmarksHeader.label;</description>
-        <toolbarbutton id="tool-bookmarks-manage" class="urlbar-button show-text button-dark" type="checkbox" autocheck="true"
-                       label="&bookmarksManage.label;" oncommand="BookmarkList.toggleManage();"/>
         <toolbarbutton id="tool-bookmarks-close" class="urlbar-button button-image" command="cmd_close"/>
       </hbox>
       <placelist id="bookmark-items" type="bookmarks" flex="1" onopen="BookmarkList.openBookmark();" autoedit="true"/>
     </vbox>
 
     <!-- options dialog for select form field -->
     <vbox id="select-container" hidden="true" pack="center">
       <spacer flex="1000"/>
@@ -515,16 +513,22 @@
       </hbox>
       <richlistbox id="context-commands" onclick="ContextHelper.hide();">
         <richlistitem id="context-openinnewtab" type="link" onclick="ContextCommands.openInNewTab(event);">
           <label value="&contextOpenInNewTab.label;"/>
         </richlistitem>
         <richlistitem id="context-saveimage" type="image-loaded" onclick="ContextCommands.saveImage(event);">
           <label value="&contextSaveImage.label;"/>
         </richlistitem>
+        <richlistitem id="context-editbookmark" type="edit-bookmark" onclick="ContextCommands.editBookmark(event);">
+          <label value="&contextEditBookmark.label;"/>
+        </richlistitem>
+        <richlistitem id="context-removebookmark" type="edit-bookmark" onclick="ContextCommands.removeBookmark(event);">
+          <label value="&contextRemoveBookmark.label;"/>
+        </richlistitem>
       </richlistbox>
     </vbox>
 
     <!-- alerts for content -->
     <hbox id="alerts-container" hidden="true" align="start" class="dialog-dark" top="0" left="0"
           onclick="AlertsHelper.click(event);">
       <image id="alerts-image"/>
       <vbox flex="1">
--- a/mobile/locales/en-US/chrome/browser.dtd
+++ b/mobile/locales/en-US/chrome/browser.dtd
@@ -16,19 +16,17 @@
 <!ENTITY copylink.label        "Copy Link Location">
 <!ENTITY paste.label           "Paste">
 <!ENTITY delete.label          "Delete">
 <!ENTITY selectAll.label       "Select All">
 <!ENTITY noSuggestions.label   "(No suggestions)">
 <!ENTITY addToDictionary.label "Add to Dictionary">
 
 <!ENTITY bookmarksHeader.label     "Bookmarks">
-<!ENTITY bookmarksManage.label     "Manage">
 
-<!ENTITY editBookmarkRemove.label  "Remove">
 <!ENTITY editBookmarkDone.label    "Done">
 <!ENTITY editBookmarkTags.label    "Add tags here">
 
 <!ENTITY selectHelper.done         "Done">
 
 <!ENTITY addonsHeader.label        "Add-ons">
 <!ENTITY addonsLocal.label         "Your Add-ons">
 <!ENTITY addonsUpdate.label        "Update">
@@ -81,10 +79,12 @@
 <!ENTITY consoleMessages.label     "Messages">
 <!ENTITY consoleCodeEval.label     "Code:">
 <!ENTITY consoleClear.label        "Clear">
 <!ENTITY consoleEvaluate.label     "…">
 <!ENTITY consoleErrFile.label      "Source File:">
 <!ENTITY consoleErrLine.label      "Line:">
 <!ENTITY consoleErrColumn.label    "Column:">
 
-<!ENTITY contextOpenInNewTab.label "Open Link in New Tab">
-<!ENTITY contextSaveImage.label    "Save Image">
+<!ENTITY contextOpenInNewTab.label    "Open Link in New Tab">
+<!ENTITY contextSaveImage.label       "Save Image">
+<!ENTITY contextEditBookmark.label    "Edit">
+<!ENTITY contextRemoveBookmark.label  "Remove">