Bug 1385083 - history panel should actually list history, r?mikedeboer draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 07 Aug 2017 15:24:49 +0100
changeset 642060 ac1549f3f69b9a54a2a54cf6a42167b7b401aa70
parent 641632 47248637eafa9a38dade8dc3aa6c4736177c8d8d
child 724891 e056a7bf9e196d182cf48eeeb5ebda56f3740e6d
push id72633
push userbmo:gijskruitbosch+bugs@gmail.com
push dateMon, 07 Aug 2017 16:23:36 +0000
reviewersmikedeboer
bugs1385083
milestone57.0a1
Bug 1385083 - history panel should actually list history, r?mikedeboer MozReview-Commit-ID: G98QMcmASv0
browser/base/content/browser-places.js
browser/components/customizableui/CustomizableWidgets.jsm
browser/components/customizableui/test/browser.ini
browser/components/customizableui/test/browser_947914_button_history.js
browser/components/customizableui/test/dummy_history_item.html
browser/components/places/content/browserPlacesViews.js
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -1983,20 +1983,16 @@ var BookmarkingUI = {
     for (let i = 0, l = staticButtons.length; i < l; ++i)
       CustomizableUI.addShortcut(staticButtons[i]);
     // Setup the Places view.
     // We restrict the amount of results to 42. Not 50, but 42. Why? Because 42.
     let query = "place:queryType=" + Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS +
       "&sort=" + Ci.nsINavHistoryQueryOptions.SORT_BY_DATEADDED_DESCENDING +
       "&maxResults=42&excludeQueries=1";
 
-    // XPCOMUtils.defineLazyScriptGetter can't return class constructors, so
-    // trigger the getter once without using the result before calling
-    // PlacesPanelview as a constructor.
-    PlacesPanelview;
     this._panelMenuView = new PlacesPanelview(document.getElementById("panelMenu_bookmarksMenu"),
       panelview, query);
     panelview.removeEventListener("ViewShowing", this);
   },
 
   onPanelMenuViewHiding: function BUI_onViewHiding(aEvent) {
     this._panelMenuView.uninit();
     delete this._panelMenuView;
--- a/browser/components/customizableui/CustomizableWidgets.jsm
+++ b/browser/components/customizableui/CustomizableWidgets.jsm
@@ -170,39 +170,40 @@ const CustomizableWidgets = [
           break;
         case "ViewShowing":
           this.onSubViewShowing(event);
           break;
         default:
           throw new Error(`Unsupported event for '${this.id}'`);
       }
     },
-    onViewShowing(aEvent) {
+    onViewShowing(event) {
       if (this._panelMenuView)
         return;
 
       let panelview = event.target;
       let document = panelview.ownerDocument;
       let window = document.defaultView;
 
       // We restrict the amount of results to 42. Not 50, but 42. Why? Because 42.
       let query = "place:queryType=" + Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY +
         "&sort=" + Ci.nsINavHistoryQueryOptions.SORT_BY_DATE_DESCENDING +
         "&maxResults=42&excludeQueries=1";
+
       this._panelMenuView = new window.PlacesPanelview(document.getElementById("appMenu_historyMenu"),
         panelview, query);
       // When either of these sub-subviews show, populate them with recently closed
       // objects data.
       document.getElementById(this.recentlyClosedTabsPanel).addEventListener("ViewShowing", this);
       document.getElementById(this.recentlyClosedWindowsPanel).addEventListener("ViewShowing", this);
       // When the popup is hidden (thus the panelmultiview node as well), make
       // sure to stop listening to PlacesDatabase updates.
       panelview.panelMultiView.addEventListener("PanelMultiViewHidden", this);
     },
-    onViewHiding(aEvent) {
+    onViewHiding(event) {
       log.debug("History view is being hidden!");
     },
     onPanelMultiViewHidden(event) {
       let panelMultiView = event.target;
       let document = panelMultiView.ownerDocument;
       if (this._panelMenuView) {
         this._panelMenuView.uninit();
         delete this._panelMenuView;
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -65,16 +65,17 @@ skip-if = os == "linux" # Intermittent f
 subsuite = clipboard
 skip-if = os == "linux" # Intermittent failures on Linux
 [browser_947914_button_cut.js]
 subsuite = clipboard
 skip-if = os == "linux" # Intermittent failures on Linux
 [browser_947914_button_find.js]
 skip-if = os == "linux" # Intermittent failures
 [browser_947914_button_history.js]
+support-files = dummy_history_item.html
 skip-if = os == "linux" # Intermittent failures
 [browser_947914_button_newPrivateWindow.js]
 skip-if = os == "linux" # Intermittent failures
 [browser_947914_button_newWindow.js]
 skip-if = os == "linux" # Intermittent failures
 [browser_947914_button_paste.js]
 subsuite = clipboard
 skip-if = os == "linux" # Intermittent failures on Linux
--- a/browser/components/customizableui/test/browser_947914_button_history.js
+++ b/browser/components/customizableui/test/browser_947914_button_history.js
@@ -1,28 +1,36 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
   * License, v. 2.0. If a copy of the MPL was not distributed with this
   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
+const TEST_PATH = getRootDirectory(gTestPath).replace("chrome://mochitests/content", "http://example.com");
+
 add_task(async function() {
   info("Check history button existence and functionality");
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_PATH + "dummy_history_item.html");
+  await BrowserTestUtils.removeTab(tab);
+
   CustomizableUI.addWidgetToArea("history-panelmenu", CustomizableUI.AREA_FIXED_OVERFLOW_PANEL);
   registerCleanupFunction(() => CustomizableUI.reset());
 
   await document.getElementById("nav-bar").overflowable.show();
   info("Menu panel was opened");
 
   let historyButton = document.getElementById("history-panelmenu");
   ok(historyButton, "History button appears in Panel Menu");
 
   let historyPanel = document.getElementById("PanelUI-history");
   let promise = BrowserTestUtils.waitForEvent(historyPanel, "ViewShown");
   historyButton.click();
   await promise;
   ok(historyPanel.getAttribute("current"), "History Panel is in view");
+  let historyItems = document.getElementById("appMenu_historyMenu");
+  ok(historyItems.querySelector("toolbarbutton.bookmark-item[label='Happy History Hero']"),
+     "Should have a history item for the history we just made.");
 
   let panelHiddenPromise = promiseOverflowHidden(window);
   document.getElementById("widget-overflow").hidePopup();
   await panelHiddenPromise
   info("Menu panel was closed");
 });
new file mode 100644
--- /dev/null
+++ b/browser/components/customizableui/test/dummy_history_item.html
@@ -0,0 +1,2 @@
+<title>Happy History Hero</title>
+<p>I am a page for the history books.</p>
--- a/browser/components/places/content/browserPlacesViews.js
+++ b/browser/components/places/content/browserPlacesViews.js
@@ -1999,25 +1999,25 @@ PlacesPanelMenuView.prototype = {
     }
 
     for (let i = 0; i < this._resultNode.childCount; ++i) {
       this._insertNewItem(this._resultNode.getChild(i), null);
     }
   }
 };
 
-var PlacesPanelview = class extends PlacesViewBase {
+this.PlacesPanelview = class extends PlacesViewBase {
   constructor(container, panelview, place, options = {}) {
     options.rootElt = container;
     options.viewElt = panelview;
     super(place, options);
     this._viewElt._placesView = this;
     // We're simulating a popup show, because a panelview may only be shown when
     // its containing popup is already shown.
-    this._onPopupShowing({ originalTarget: this._viewElt });
+    this._onPopupShowing({ originalTarget: this._rootElt });
     this._addEventListeners(window, ["unload"]);
     this._rootElt.setAttribute("context", "placesContext");
   }
 
   get events() {
     if (this._events)
       return this._events;
     return this._events = ["command", "destructed", "dragend", "dragstart",