From e43023925b86ae373bbe8c987665f53c7c4a9e82 Mon Sep 17 00:00:00 2001
From: Gijs Kruitbosch <gijskruitbosch@gmail.com>
Date: Mon, 3 Jun 2013 20:59:31 +0200
Subject: [PATCH] Bug 870452 - Migrate feed button to new API, r=mconley

---
 browser/base/content/browser-feeds.js         | 64 +++++++------------
 browser/base/content/browser.css              |  6 +-
 browser/base/content/browser.xul              | 14 ----
 .../customizableui/content/panelUI.inc.xul    |  2 +
 .../customizableui/content/panelUI.js         |  3 +
 .../customizableui/content/panelUI.xml        |  3 +
 .../customizableui/src/CustomizableUI.jsm     | 38 +++++++++--
 .../src/CustomizableWidgets.jsm               | 41 ++++++++++++
 .../locales/en-US/chrome/browser/browser.dtd  |  3 -
 .../customizableWidgets.properties            |  3 +
 10 files changed, 110 insertions(+), 67 deletions(-)

diff --git a/browser/base/content/browser-feeds.js b/browser/base/content/browser-feeds.js
index 7bba39540824..5b432ede2f94 100644
--- a/browser/base/content/browser-feeds.js
+++ b/browser/base/content/browser-feeds.js
@@ -8,66 +8,50 @@
  * and shows UI when they are discovered.
  */
 var FeedHandler = {
-  /**
-   * The click handler for the Feed icon in the toolbar. Opens the
-   * subscription page if user is not given a choice of feeds.
-   * (Otherwise the list of available feeds will be presented to the
-   * user in a popup menu.)
-   */
-  onFeedButtonClick: function(event) {
-    event.stopPropagation();
-
-    let feeds = gBrowser.selectedBrowser.feeds || [];
-    // If there are multiple feeds, the menu will open, so no need to do
-    // anything. If there are no feeds, nothing to do either.
-    if (feeds.length != 1)
-      return;
-
-    if (event.eventPhase == Event.AT_TARGET &&
-        (event.button == 0 || event.button == 1)) {
-      this.subscribeToFeed(feeds[0].href, event);
-    }
-  },
-
- /** Called when the user clicks on the Subscribe to This Page... menu item.
+  /** Called when the user clicks on the Subscribe to This Page... menu item,
+   * or when the user clicks the feed button when the page contains multiple
+   * feeds.
    * Builds a menu of unique feeds associated with the page, and if there
    * is only one, shows the feed inline in the browser window.
-   * @param   menuPopup
-   *          The feed list menupopup to be populated.
-   * @returns true if the menu should be shown, false if there was only
+   * @param   container
+   *          The feed list container (menupopup or subview) to be populated.
+   * @param   isSubview
+   *          Whether we're creating a subview (true) or menu (false/undefined)
+   * @returns true if the menu/subview should be shown, false if there was only
    *          one feed and the feed should be shown inline in the browser
-   *          window (do not show the menupopup).
+   *          window (do not show the menupopup/subview).
    */
-  buildFeedList: function(menuPopup) {
+  buildFeedList: function(container, isSubview) {
     var feeds = gBrowser.selectedBrowser.feeds;
-    if (feeds == null) {
+    if (!isSubview && feeds == null) {
       // XXX hack -- menu opening depends on setting of an "open"
       // attribute, and the menu refuses to open if that attribute is
       // set (because it thinks it's already open).  onpopupshowing gets
       // called after the attribute is unset, and it doesn't get unset
       // if we return false.  so we unset it here; otherwise, the menu
       // refuses to work past this point.
-      menuPopup.parentNode.removeAttribute("open");
+      container.parentNode.removeAttribute("open");
       return false;
     }
 
-    while (menuPopup.firstChild)
-      menuPopup.removeChild(menuPopup.firstChild);
+    while (container.firstChild)
+      container.removeChild(container.firstChild);
 
-    if (feeds.length <= 1)
+    if (!feeds || feeds.length <= 1)
       return false;
 
     // Build the menu showing the available feed choices for viewing.
+    var itemNodeType = isSubview ? "toolbarbutton" : "menuitem";
     for (let feedInfo of feeds) {
-      var menuItem = document.createElement("menuitem");
+      var item = document.createElement(itemNodeType);
       var baseTitle = feedInfo.title || feedInfo.href;
       var labelStr = gNavigatorBundle.getFormattedString("feedShowFeedNew", [baseTitle]);
-      menuItem.setAttribute("class", "feed-menuitem");
-      menuItem.setAttribute("label", labelStr);
-      menuItem.setAttribute("feed", feedInfo.href);
-      menuItem.setAttribute("tooltiptext", feedInfo.href);
-      menuItem.setAttribute("crop", "center");
-      menuPopup.appendChild(menuItem);
+      item.setAttribute("class", "feed-" + itemNodeType);
+      item.setAttribute("label", labelStr);
+      item.setAttribute("feed", feedInfo.href);
+      item.setAttribute("tooltiptext", feedInfo.href);
+      item.setAttribute("crop", "center");
+      container.appendChild(item);
     }
     return true;
   },
@@ -76,7 +60,7 @@ var FeedHandler = {
    * Subscribe to a given feed.  Called when
    *   1. Page has a single feed and user clicks feed icon in location bar
    *   2. Page has a single feed and user selects Subscribe menu item
-   *   3. Page has multiple feeds and user selects from feed icon popup
+   *   3. Page has multiple feeds and user selects from feed icon popup (or subview)
    *   4. Page has multiple feeds and user selects from Subscribe submenu
    * @param   href
    *          The feed to subscribe to. May be null, in which case the
diff --git a/browser/base/content/browser.css b/browser/base/content/browser.css
index 0f140e4a5cae..d82c6fdcbde4 100644
--- a/browser/base/content/browser.css
+++ b/browser/base/content/browser.css
@@ -183,11 +183,7 @@ toolbar[overflowable]:not([overflowing]) > .overflow-button {
   visibility: collapse;
 }
 
-#feed-button > .toolbarbutton-menu-dropmarker {
-  display: none;
-}
-
-#feed-menu > .feed-menuitem:-moz-locale-dir(rtl) {
+#PanelUI-feeds > .feed-toolbarbutton:-moz-locale-dir(rtl) {
   direction: rtl;
 }
 
diff --git a/browser/base/content/browser.xul b/browser/base/content/browser.xul
index 940f289c4914..2922952d9b2c 100644
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -998,20 +998,6 @@
                      label="&fullScreenCmd.label;"
                      tooltiptext="&fullScreenButton.tooltip;"/>
 
-      <toolbarbutton id="feed-button"
-                     type="menu"
-                     class="toolbarbutton-1 chromeclass-toolbar-additional"
-                     disabled="true"
-                     label="&feedButton.label;"
-                     tooltiptext="&feedButton.tooltip;"
-                     onclick="return FeedHandler.onFeedButtonClick(event);">
-        <menupopup position="after_end"
-                   id="feed-menu"
-                   onpopupshowing="return FeedHandler.buildFeedList(this);"
-                   oncommand="return FeedHandler.subscribeToFeed(null, event);"
-                   onclick="checkForMiddleClick(this, event);"/>
-      </toolbarbutton>
-
 #ifdef MOZ_SERVICES_SYNC
       <toolbarbutton id="sync-button"
                      class="toolbarbutton-1 chromeclass-toolbar-additional"
diff --git a/browser/components/customizableui/content/panelUI.inc.xul b/browser/components/customizableui/content/panelUI.inc.xul
index a4be3b4326cd..5019b3fb01ba 100644
--- a/browser/components/customizableui/content/panelUI.inc.xul
+++ b/browser/components/customizableui/content/panelUI.inc.xul
@@ -40,6 +40,8 @@
              onclick="PlacesCommandHook.showPlacesOrganizer('History'); PanelUI.hide();"/>
     </panelview>
 
+    <panelview id="PanelUI-feeds" flex="1" oncommand="FeedHandler.subscribeToFeed(null, event);"></panelview>
+
     <panelview id="PanelUI-help" flex="1">
       <label value="&helpMenu.label;"/>
       <vbox id="PanelUI-helpItems"/>
diff --git a/browser/components/customizableui/content/panelUI.js b/browser/components/customizableui/content/panelUI.js
index 6feee58def75..da727db1b478 100644
--- a/browser/components/customizableui/content/panelUI.js
+++ b/browser/components/customizableui/content/panelUI.js
@@ -162,6 +162,9 @@ const PanelUI = {
       let evt = document.createEvent("CustomEvent");
       evt.initCustomEvent("ViewShowing", true, true, viewNode);
       viewNode.dispatchEvent(evt);
+      if (evt.defaultPrevented) {
+        return;
+      }
 
       let tempPanel = document.createElement("panel");
       tempPanel.setAttribute("type", "arrow");
diff --git a/browser/components/customizableui/content/panelUI.xml b/browser/components/customizableui/content/panelUI.xml
index dcb86ae2bb39..b40f05cd8455 100644
--- a/browser/components/customizableui/content/panelUI.xml
+++ b/browser/components/customizableui/content/panelUI.xml
@@ -191,6 +191,9 @@
           let evt = document.createEvent("CustomEvent");
           evt.initCustomEvent("ViewShowing", true, true, viewNode);
           viewNode.dispatchEvent(evt);
+          if (evt.defaultPrevented) {
+            return;
+          }
 
           this._currentSubView = viewNode;
 
diff --git a/browser/components/customizableui/src/CustomizableUI.jsm b/browser/components/customizableui/src/CustomizableUI.jsm
index a8cd3afac178..f97f6de03c9a 100644
--- a/browser/components/customizableui/src/CustomizableUI.jsm
+++ b/browser/components/customizableui/src/CustomizableUI.jsm
@@ -768,8 +768,10 @@ let CustomizableUIInternal = {
       }
       node.setAttribute("class", "toolbarbutton-1 chromeclass-toolbar-additional");
 
-      let handler = this.handleWidgetClick.bind(this, aWidget, node);
-      node.addEventListener("command", handler, false);
+      let commandHandler = this.handleWidgetCommand.bind(this, aWidget, node);
+      node.addEventListener("command", commandHandler, false);
+      let clickHandler = this.handleWidgetClick.bind(this, aWidget, node);
+      node.addEventListener("click", clickHandler, false);
 
       // If the widget has a view, and has view showing / hiding listeners,
       // hook those up to this widget.
@@ -796,6 +798,10 @@ let CustomizableUIInternal = {
 
         LOG("Widget " + aWidget.id + " showing and hiding event handlers set.");
       }
+
+      if (aWidget.onCreated) {
+        aWidget.onCreated(node);
+      }
     }
 
     aWidget.instances.set(aDocument, node);
@@ -826,8 +832,8 @@ let CustomizableUIInternal = {
     return def;
   },
 
-  handleWidgetClick: function(aWidget, aNode, aEvent) {
-    LOG("handleWidgetClick");
+  handleWidgetCommand: function(aWidget, aNode, aEvent) {
+    LOG("handleWidgetCommand");
 
     if (aWidget.type == "button") {
       this.maybeAutoHidePanel(aEvent);
@@ -841,7 +847,7 @@ let CustomizableUIInternal = {
       } else {
         //XXXunf Need to think this through more, and formalize.
         Services.obs.notifyObservers(aNode,
-                                     "customizedui-widget-click",
+                                     "customizedui-widget-command",
                                      aWidget.id);
       }
     } else if (aWidget.type == "view") {
@@ -851,6 +857,24 @@ let CustomizableUIInternal = {
     }
   },
 
+  handleWidgetClick: function(aWidget, aNode, aEvent) {
+    LOG("handleWidgetClick");
+    if (aWidget.type == "button") {
+      this.maybeAutoHidePanel(aEvent);
+    }
+
+    if (aWidget.onClick) {
+      try {
+        aWidget.onClick.call(null, aEvent);
+      } catch(e) {
+        Cu.reportError(e);
+      }
+    } else {
+      //XXXunf Need to think this through more, and formalize.
+      Services.obs.notifyObservers(aNode, "customizedui-widget-click", aWidget.id);
+    }
+  },
+
   maybeAutoHidePanel: function(aEvent) {
     if (aEvent.type == "keypress" && 
         !(aEvent.keyCode == aEvent.DOM_VK_ENTER ||
@@ -1361,6 +1385,10 @@ let CustomizableUIInternal = {
       }
     }
 
+    widget.onClick = typeof aData.onClick == "function" ? aData.onClick : null;
+
+    widget.onCreated = typeof aData.onCreated == "function" ? aData.onCreated : null;
+
     if (widget.type == "button") {
       widget.onCommand = typeof aData.onCommand == "function" ?
                            aData.onCommand :
diff --git a/browser/components/customizableui/src/CustomizableWidgets.jsm b/browser/components/customizableui/src/CustomizableWidgets.jsm
index 2d5654c589d1..226035c0052a 100644
--- a/browser/components/customizableui/src/CustomizableWidgets.jsm
+++ b/browser/components/customizableui/src/CustomizableWidgets.jsm
@@ -450,4 +450,45 @@ const CustomizableWidgets = [{
 
       return node;
     }
+  },
+  {
+    id: "feed-button",
+    type: "view",
+    viewId: "PanelUI-feeds",
+    removable: true,
+    defaultArea: CustomizableUI.AREA_PANEL,
+    allowedAreas: [CustomizableUI.AREA_PANEL, CustomizableUI.AREA_NAVBAR],
+    onClick: function(aEvent) {
+      let win = aEvent.target.ownerDocument.defaultView;
+      let feeds = win.gBrowser.selectedBrowser.feeds;
+
+      // Here, we only care about the case where we have exactly 1 feed and the
+      // user clicked...
+      let isClick = (aEvent.button == 0 || aEvent.button == 1);
+      if (feeds && feeds.length == 1 && isClick) {
+        aEvent.preventDefault();
+        aEvent.stopPropagation();
+        win.FeedHandler.subscribeToFeed(feeds[0].href, aEvent);
+      }
+    },
+    onViewShowing: function(aEvent) {
+      let doc = aEvent.detail.ownerDocument;
+      let container = doc.getElementById("PanelUI-feeds");
+      let gotView = doc.defaultView.FeedHandler.buildFeedList(container, true);
+
+      // For no feeds or only a single one, don't show the panel.
+      if (!gotView) {
+        aEvent.preventDefault();
+        aEvent.stopPropagation();
+        return;
+      }
+    },
+    onCreated: function(node) {
+      let win = node.ownerDocument.defaultView;
+      let selectedBrowser = win.gBrowser.selectedBrowser;
+      let feeds = selectedBrowser && selectedBrowser.feeds;
+      if (!feeds || !feeds.length) {
+        node.setAttribute("disabled", "true");
+      }
+    }
   }];
diff --git a/browser/locales/en-US/chrome/browser/browser.dtd b/browser/locales/en-US/chrome/browser/browser.dtd
index 2b2ad0bd111f..5cf8817a4570 100644
--- a/browser/locales/en-US/chrome/browser/browser.dtd
+++ b/browser/locales/en-US/chrome/browser/browser.dtd
@@ -166,9 +166,6 @@ These should match what Safari and other Apple applications use on OS X Lion. --
 <!ENTITY tabGroupsButton.label        "Tab Groups">
 <!ENTITY tabGroupsButton.tooltip      "Group your tabs">
 
-<!ENTITY feedButton.label             "Subscribe">
-<!ENTITY feedButton.tooltip           "Subscribe to this page…">
-
 <!ENTITY bookmarksButton.label          "Bookmarks">
 <!ENTITY bookmarksCmd.commandkey "b">
 
diff --git a/browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties b/browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties
index af4bb723b1cf..e1c7a36c4b97 100644
--- a/browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties
+++ b/browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties
@@ -62,3 +62,6 @@ copy-button.tooltiptext = Copy
 
 paste-button.label = Paste
 paste-button.tooltiptext = Paste
+
+feed-button.label = Subscribe
+feed-button.tooltiptext = Subscribe to this page…