Bug 998163 - 8% Tp5 Optimized regression on inbound for most all platforms [r=adw]
authorEdward Lee <edilee@mozilla.com>
Fri, 18 Apr 2014 13:58:24 -0700
changeset 179286 5c8763cd059df9396e586d378679bc624ab73a15
parent 179285 f2e5bc417f969d31a5c1eba73cb5fef0391e36d9
child 179287 f8118ec22ed9b560f1503036a09d5a0d04ac9c41
push id26612
push userryanvm@gmail.com
push dateSat, 19 Apr 2014 01:50:19 +0000
treeherdermozilla-central@c98998672a15 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs998163, 998387
milestone31.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 998163 - 8% Tp5 Optimized regression on inbound for most all platforms [r=adw] Bug 998387 - Middle click on newtab block button blocks the site Only add click listener once at the page level and filter/delegate click handling looking for primary/middle clicks.
browser/base/content/newtab/page.js
browser/base/content/newtab/sites.js
browser/base/content/test/newtab/browser.ini
browser/base/content/test/newtab/browser_newtab_bug991111.js
browser/base/content/test/newtab/browser_newtab_bug998387.js
--- a/browser/base/content/newtab/page.js
+++ b/browser/base/content/newtab/page.js
@@ -14,19 +14,20 @@ let gPage = {
    */
   init: function Page_init() {
     // Add ourselves to the list of pages to receive notifications.
     gAllPages.register(this);
 
     // Listen for 'unload' to unregister this page.
     addEventListener("unload", this, false);
 
-    // Listen for toggle button clicks.
-    let button = document.getElementById("newtab-toggle");
-    button.addEventListener("click", this, false);
+    // XXX bug 991111 - Not all click events are correctly triggered when
+    // listening from xhtml nodes -- in particular middle clicks on sites, so
+    // listen from the xul window and filter then delegate
+    addEventListener("click", this, false);
 
     // Initialize sponsored panel
     this._sponsoredPanel = document.getElementById("sponsored-panel");
     let link = this._sponsoredPanel.querySelector(".text-link");
     link.addEventListener("click", () => this._sponsoredPanel.hidePopup());
 
     // Check if the new tab feature is enabled.
     let enabled = gAllPages.enabled;
@@ -191,17 +192,32 @@ let gPage = {
   handleEvent: function Page_handleEvent(aEvent) {
     switch (aEvent.type) {
       case "unload":
         if (this._mutationObserver)
           this._mutationObserver.disconnect();
         gAllPages.unregister(this);
         break;
       case "click":
-        gAllPages.enabled = !gAllPages.enabled;
+        let {button, target} = aEvent;
+        if (target.id == "newtab-toggle") {
+          if (button == 0) {
+            gAllPages.enabled = !gAllPages.enabled;
+          }
+          break;
+        }
+
+        // Go up ancestors until we find a Site or not
+        while (target) {
+          if (target.hasOwnProperty("_newtabSite")) {
+            target._newtabSite.onClick(aEvent);
+            break;
+          }
+          target = target.parentNode;
+        }
         break;
       case "dragover":
         if (gDrag.isValid(aEvent) && gDrag.draggedSite)
           aEvent.preventDefault();
         break;
       case "drop":
         if (gDrag.isValid(aEvent) && gDrag.draggedSite) {
           aEvent.preventDefault();
--- a/browser/base/content/newtab/sites.js
+++ b/browser/base/content/newtab/sites.js
@@ -165,20 +165,16 @@ Site.prototype = {
    * Adds event handlers for the site and its buttons.
    */
   _addEventHandlers: function Site_addEventHandlers() {
     // Register drag-and-drop event handlers.
     this._node.addEventListener("dragstart", this, false);
     this._node.addEventListener("dragend", this, false);
     this._node.addEventListener("mouseover", this, false);
 
-    // XXX bug 991111 - Not all click events are correctly triggered when
-    // listening from the xhtml node, so listen from the xul window and filter
-    addEventListener("click", this, false);
-
     // Specially treat the sponsored icon to prevent regular hover effects
     let sponsored = this._querySelector(".newtab-control-sponsored");
     sponsored.addEventListener("mouseover", () => {
       this.cell.node.setAttribute("ignorehover", "true");
     });
     sponsored.addEventListener("mouseout", () => {
       this.cell.node.removeAttribute("ignorehover");
     });
@@ -213,21 +209,29 @@ Site.prototype = {
       Services.telemetry.getHistogramById("NEWTAB_PAGE_DIRECTORY_TYPE_CLICKED")
                         .add(typeIndex);
     }
   },
 
   /**
    * Handles site click events.
    */
-  _onClick: function Site_onClick(aEvent) {
-    let target = aEvent.target;
+  onClick: function Site_onClick(aEvent) {
+    let {button, target} = aEvent;
     if (target.classList.contains("newtab-link") ||
         target.parentElement.classList.contains("newtab-link")) {
-      this._recordSiteClicked(this.cell.index);
+      // Record for primary and middle clicks
+      if (button == 0 || button == 1) {
+        this._recordSiteClicked(this.cell.index);
+      }
+      return;
+    }
+
+    // Only handle primary clicks for the remaining targets
+    if (button != 0) {
       return;
     }
 
     aEvent.preventDefault();
     if (aEvent.target.classList.contains("newtab-control-block"))
       this.block();
     else if (target.classList.contains("newtab-control-sponsored"))
       gPage.showSponsoredPanel(target);
@@ -237,23 +241,16 @@ Site.prototype = {
       this.pin();
   },
 
   /**
    * Handles all site events.
    */
   handleEvent: function Site_handleEvent(aEvent) {
     switch (aEvent.type) {
-      case "click":
-        // Check the bitmask if the click event is for the site's descendants
-        if (this._node.compareDocumentPosition(aEvent.target) &
-            this._node.DOCUMENT_POSITION_CONTAINED_BY) {
-          this._onClick(aEvent);
-        }
-        break;
       case "mouseover":
         this._node.removeEventListener("mouseover", this, false);
         this._speculativeConnect();
         break;
       case "dragstart":
         gDrag.start(this, aEvent);
         break;
       case "dragend":
--- a/browser/base/content/test/newtab/browser.ini
+++ b/browser/base/content/test/newtab/browser.ini
@@ -12,16 +12,17 @@ skip-if = e10s # Bug ?????? - about:newt
 [browser_newtab_bug734043.js]
 [browser_newtab_bug735987.js]
 skip-if = os == "mac" # Intermittent failures, bug 898317
 [browser_newtab_bug752841.js]
 [browser_newtab_bug765628.js]
 [browser_newtab_bug876313.js]
 [browser_newtab_bug991111.js]
 [browser_newtab_bug991210.js]
+[browser_newtab_bug998387.js]
 [browser_newtab_disable.js]
 [browser_newtab_drag_drop.js]
 [browser_newtab_drag_drop_ext.js]
 [browser_newtab_drop_preview.js]
 [browser_newtab_focus.js]
 [browser_newtab_perwindow_private_browsing.js]
 [browser_newtab_reset.js]
 [browser_newtab_sponsored_icon_click.js]
--- a/browser/base/content/test/newtab/browser_newtab_bug991111.js
+++ b/browser/base/content/test/newtab/browser_newtab_bug991111.js
@@ -3,17 +3,17 @@
 
 function runTests() {
   yield setLinks("0");
   yield addNewTabPageTab();
 
   // Remember if the click handler was triggered
   let cell = getCell(0);
   let clicked = false;
-  cell.site._onClick = e => {
+  cell.site.onClick = e => {
     clicked = true;
     executeSoon(TestRunner.next);
   };
 
   // Send a middle-click and make sure it happened
   yield EventUtils.synthesizeMouseAtCenter(cell.node, {button: 1}, getContentWindow());
   ok(clicked, "middle click triggered click listener");
 }
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/newtab/browser_newtab_bug998387.js
@@ -0,0 +1,23 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+function runTests() {
+  yield setLinks("0");
+  yield addNewTabPageTab();
+
+  // Remember if the click handler was triggered
+  let cell = getCell(0);
+  let clicked = false;
+  cell.site.onClick = e => {
+    clicked = true;
+    executeSoon(TestRunner.next);
+  };
+
+  // Send a middle-click and make sure it happened
+  let block = getContentDocument().querySelector(".newtab-control-block");
+  yield EventUtils.synthesizeMouseAtCenter(block, {button: 1}, getContentWindow());
+  ok(clicked, "middle click triggered click listener");
+
+  // Make sure the cell didn't actually get blocked
+  checkGrid("0");
+}