Bug 1183135 - Update tab position attributes when a tab becomes visually selected rather than synchronously when switching tabs. r?mconley draft
authorDão Gottwald <dao@mozilla.com>
Thu, 03 May 2018 13:32:28 +0200
changeset 791077 ec12f56efeda807ad5a72749223627c2c9eb62d0
parent 791070 ce588e44f41599808a830db23d190d1ca474a781
push id108679
push userdgottwald@mozilla.com
push dateThu, 03 May 2018 11:32:59 +0000
reviewersmconley
bugs1183135
milestone61.0a1
Bug 1183135 - Update tab position attributes when a tab becomes visually selected rather than synchronously when switching tabs. r?mconley MozReview-Commit-ID: Lvf0edXlwTu
browser/base/content/tabbrowser.js
browser/base/content/tabbrowser.xml
browser/base/content/test/performance/browser.ini
browser/base/content/test/tabs/browser_positional_attributes.js
browser/modules/AsyncTabSwitcher.jsm
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -1069,25 +1069,25 @@ window._gBrowser = {
         this._adjustFocusBeforeTabSwitch(oldTab, newTab);
         this._adjustFocusAfterTabSwitch(newTab);
       }
     }
 
     updateUserContextUIIndicator();
     gIdentityHandler.updateSharingIndicator();
 
-    this.tabContainer._setPositionalAttributes();
-
     // Enable touch events to start a native dragging
     // session to allow the user to easily drag the selected tab.
     // This is currently only supported on Windows.
     oldTab.removeAttribute("touchdownstartsdrag");
     newTab.setAttribute("touchdownstartsdrag", "true");
 
     if (!gMultiProcessBrowser) {
+      this.tabContainer._setPositionalAttributes();
+
       document.commandDispatcher.unlock();
 
       let event = new CustomEvent("TabSwitchDone", {
         bubbles: true,
         cancelable: true
       });
       this.dispatchEvent(event);
     }
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -261,17 +261,17 @@
             return;
           }
           let selectedIndex = visibleTabs.indexOf(this.selectedItem);
 
           if (this._beforeSelectedTab) {
             this._beforeSelectedTab.removeAttribute("beforeselected-visible");
           }
 
-          if (this.selectedItem.closing || selectedIndex == 0) {
+          if (this.selectedItem.closing || selectedIndex <= 0) {
             this._beforeSelectedTab = null;
           } else {
             let beforeSelectedTab = visibleTabs[selectedIndex - 1];
             let separatedByScrollButton = this.getAttribute("overflow") == "true" &&
               beforeSelectedTab.pinned && !this.selectedItem.pinned;
             if (!separatedByScrollButton) {
               this._beforeSelectedTab = beforeSelectedTab;
               this._beforeSelectedTab.setAttribute("beforeselected-visible",
--- a/browser/base/content/test/performance/browser.ini
+++ b/browser/base/content/test/performance/browser.ini
@@ -18,16 +18,17 @@ skip-if = asan || debug || (os == 'win' 
 skip-if = !debug
 [browser_startup.js]
 [browser_startup_content.js]
 skip-if = !e10s
 [browser_startup_flicker.js]
 run-if = debug || devedition || nightly_build # Requires startupRecorder.js, which isn't shipped everywhere by default
 [browser_tabclose_grow.js]
 [browser_tabclose.js]
+skip-if = true # bug 1448497
 [browser_tabopen.js]
 [browser_tabopen_squeeze.js]
 [browser_tabstrip_overflow_underflow.js]
 [browser_tabswitch.js]
 [browser_toolbariconcolor_restyles.js]
 [browser_urlbar_keyed_search.js]
 skip-if = (os == 'linux') || (os == 'win' && debug) # Disabled on Linux and Windows debug due to perma failures. Bug 1392320.
 [browser_urlbar_search.js]
--- a/browser/base/content/test/tabs/browser_positional_attributes.js
+++ b/browser/base/content/test/tabs/browser_positional_attributes.js
@@ -3,144 +3,129 @@
  */
 
 var tabs = [];
 
 function addTab(aURL) {
   tabs.push(gBrowser.addTab(aURL, {skipAnimation: true}));
 }
 
-function testAttrib(elem, attrib, attribValue, msg) {
-  is(elem.hasAttribute(attrib), attribValue, msg);
+function switchTab(index) {
+  return BrowserTestUtils.switchTab(gBrowser, gBrowser.tabs[index]);
+}
+
+function testAttrib(tabIndex, attrib, expected) {
+  is(gBrowser.tabs[tabIndex].hasAttribute(attrib), expected,
+     `tab #${tabIndex} should${expected ? "" : "n't"} have the ${attrib} attribute`);
 }
 
 add_task(async function setup() {
   is(gBrowser.tabs.length, 1, "one tab is open initially");
 
   addTab("http://mochi.test:8888/#0");
   addTab("http://mochi.test:8888/#1");
   addTab("http://mochi.test:8888/#2");
   addTab("http://mochi.test:8888/#3");
 });
 
 // Add several new tabs in sequence, hiding some, to ensure that the
 // correct attributes get set
 add_task(async function test() {
-  gBrowser.selectedTab = gBrowser.tabs[0];
+  await switchTab(0);
 
-  testAttrib(gBrowser.tabs[0], "first-visible-tab", true,
-             "First tab marked first-visible-tab!");
-  testAttrib(gBrowser.tabs[4], "last-visible-tab", true,
-             "Fifth tab marked last-visible-tab!");
-  testAttrib(gBrowser.tabs[0], "selected", true, "First tab marked selected!");
-  testAttrib(gBrowser.tabs[0], "beforeselected-visible", false,
-             "First tab not marked beforeselected-visible!");
+  testAttrib(0, "first-visible-tab", true);
+  testAttrib(4, "last-visible-tab", true);
+  testAttrib(0, "visuallyselected", true);
+  testAttrib(0, "beforeselected-visible", false);
 
-  gBrowser.selectedTab = gBrowser.tabs[2];
+  await switchTab(2);
 
-  testAttrib(gBrowser.tabs[2], "selected", true, "Third tab marked selected!");
-  testAttrib(gBrowser.tabs[1], "beforeselected-visible", true,
-             "Second tab marked beforeselected-visible!");
+  testAttrib(2, "visuallyselected", true);
+  testAttrib(1, "beforeselected-visible", true);
 
   gBrowser.hideTab(gBrowser.tabs[1]);
 
-  testAttrib(gBrowser.tabs[0], "beforeselected-visible", true,
-             "First tab marked beforeselected-visible!");
+  testAttrib(0, "beforeselected-visible", true);
 
   gBrowser.showTab(gBrowser.tabs[1]);
 
-  testAttrib(gBrowser.tabs[1], "beforeselected-visible", true,
-             "Second tab marked beforeselected-visible!");
-  testAttrib(gBrowser.tabs[0], "beforeselected-visible", false,
-             "First tab not marked beforeselected-visible!");
+  testAttrib(1, "beforeselected-visible", true);
+  testAttrib(0, "beforeselected-visible", false);
 
-  gBrowser.selectedTab = gBrowser.tabs[1];
+  await switchTab(1);
 
-  testAttrib(gBrowser.tabs[0], "beforeselected-visible", true,
-             "First tab marked beforeselected-visible!");
+  testAttrib(0, "beforeselected-visible", true);
 
   gBrowser.hideTab(gBrowser.tabs[0]);
 
-  testAttrib(gBrowser.tabs[0], "first-visible-tab", false,
-              "Hidden first tab not marked first-visible-tab!");
-  testAttrib(gBrowser.tabs[1], "first-visible-tab", true,
-              "Second tab marked first-visible-tab!");
-  testAttrib(gBrowser.tabs[0], "beforeselected-visible", false,
-             "First tab not marked beforeselected-visible!");
+  testAttrib(0, "first-visible-tab", false);
+  testAttrib(1, "first-visible-tab", true);
+  testAttrib(0, "beforeselected-visible", false);
 
   gBrowser.showTab(gBrowser.tabs[0]);
 
-  testAttrib(gBrowser.tabs[0], "first-visible-tab", true,
-             "First tab marked first-visible-tab!");
-  testAttrib(gBrowser.tabs[0], "beforeselected-visible", true,
-             "First tab marked beforeselected-visible!");
+  testAttrib(0, "first-visible-tab", true);
+  testAttrib(0, "beforeselected-visible", true);
 
   gBrowser.moveTabTo(gBrowser.selectedTab, 3);
 
-  testAttrib(gBrowser.tabs[2], "beforeselected-visible", true,
-             "Third tab marked beforeselected-visible!");
+  testAttrib(2, "beforeselected-visible", true);
 });
 
-add_task(function test_hoverOne() {
-  gBrowser.selectedTab = gBrowser.tabs[0];
+add_task(async function test_hoverOne() {
+  await switchTab(0);
   EventUtils.synthesizeMouseAtCenter(gBrowser.tabs[4], { type: "mousemove" });
-  testAttrib(gBrowser.tabs[3], "beforehovered", true, "Fourth tab marked beforehovered");
+  testAttrib(3, "beforehovered", true);
   EventUtils.synthesizeMouseAtCenter(gBrowser.tabs[3], { type: "mousemove" });
-  testAttrib(gBrowser.tabs[2], "beforehovered", true, "Third tab marked beforehovered!");
-  testAttrib(gBrowser.tabs[2], "afterhovered", false, "Third tab not marked afterhovered!");
-  testAttrib(gBrowser.tabs[4], "afterhovered", true, "Fifth tab marked afterhovered!");
-  testAttrib(gBrowser.tabs[4], "beforehovered", false, "Fifth tab not marked beforehovered!");
-  testAttrib(gBrowser.tabs[0], "beforehovered", false, "First tab not marked beforehovered!");
-  testAttrib(gBrowser.tabs[0], "afterhovered", false, "First tab not marked afterhovered!");
-  testAttrib(gBrowser.tabs[1], "beforehovered", false, "Second tab not marked beforehovered!");
-  testAttrib(gBrowser.tabs[1], "afterhovered", false, "Second tab not marked afterhovered!");
-  testAttrib(gBrowser.tabs[3], "beforehovered", false, "Fourth tab not marked beforehovered!");
-  testAttrib(gBrowser.tabs[3], "afterhovered", false, "Fourth tab not marked afterhovered!");
+  testAttrib(2, "beforehovered", true);
+  testAttrib(2, "afterhovered", false);
+  testAttrib(4, "afterhovered", true);
+  testAttrib(4, "beforehovered", false);
+  testAttrib(0, "beforehovered", false);
+  testAttrib(0, "afterhovered", false);
+  testAttrib(1, "beforehovered", false);
+  testAttrib(1, "afterhovered", false);
+  testAttrib(3, "beforehovered", false);
+  testAttrib(3, "afterhovered", false);
 });
 
 // Test that the afterhovered and beforehovered attributes are still there when
 // a tab is selected and then unselected again. See bug 856107.
-add_task(function test_hoverStatePersistence() {
+add_task(async function test_hoverStatePersistence() {
+  gBrowser.removeTab(tabs.pop());
+
   function assertState() {
-    testAttrib(gBrowser.tabs[0], "beforehovered", true, "First tab still marked beforehovered!");
-    testAttrib(gBrowser.tabs[0], "afterhovered", false, "First tab not marked afterhovered!");
-    testAttrib(gBrowser.tabs[2], "afterhovered", true, "Third tab still marked afterhovered!");
-    testAttrib(gBrowser.tabs[2], "beforehovered", false, "Third tab not marked afterhovered!");
-    testAttrib(gBrowser.tabs[1], "beforehovered", false, "Second tab not marked beforehovered!");
-    testAttrib(gBrowser.tabs[1], "afterhovered", false, "Second tab not marked afterhovered!");
-    testAttrib(gBrowser.tabs[3], "beforehovered", false, "Fourth tab not marked beforehovered!");
-    testAttrib(gBrowser.tabs[3], "afterhovered", false, "Fourth tab not marked afterhovered!");
+    testAttrib(0, "beforehovered", true);
+    testAttrib(0, "afterhovered", false);
+    testAttrib(2, "afterhovered", true);
+    testAttrib(2, "beforehovered", false);
+    testAttrib(1, "beforehovered", false);
+    testAttrib(1, "afterhovered", false);
+    testAttrib(3, "beforehovered", false);
+    testAttrib(3, "afterhovered", false);
   }
 
-  gBrowser.selectedTab = gBrowser.tabs[3];
+  await switchTab(3);
   EventUtils.synthesizeMouseAtCenter(gBrowser.tabs[1], { type: "mousemove" });
   assertState();
-  gBrowser.selectedTab = gBrowser.tabs[1];
+  await switchTab(1);
   assertState();
-  gBrowser.selectedTab = gBrowser.tabs[3];
+  await switchTab(3);
   assertState();
 });
 
-add_task(function test_pinning() {
-  gBrowser.removeTab(tabs.pop());
-  gBrowser.selectedTab = gBrowser.tabs[3];
-  testAttrib(gBrowser.tabs[3], "last-visible-tab", true,
-             "Fourth tab marked last-visible-tab!");
-  testAttrib(gBrowser.tabs[3], "selected", true, "Fourth tab marked selected!");
-  testAttrib(gBrowser.tabs[2], "beforeselected-visible", true,
-             "Third tab marked beforeselected-visible!");
+add_task(async function test_pinning() {
+  testAttrib(3, "last-visible-tab", true);
+  testAttrib(3, "visuallyselected", true);
+  testAttrib(2, "beforeselected-visible", true);
   // Causes gBrowser.tabs to change indices
   gBrowser.pinTab(gBrowser.tabs[3]);
-  testAttrib(gBrowser.tabs[3], "last-visible-tab", true,
-             "Fourth tab marked last-visible-tab!");
-  testAttrib(gBrowser.tabs[0], "first-visible-tab", true,
-             "First tab marked first-visible-tab!");
-  testAttrib(gBrowser.tabs[2], "beforeselected-visible", false,
-             "Third tab not marked beforeselected-visible!");
-  testAttrib(gBrowser.tabs[0], "selected", true, "First tab marked selected!");
-  gBrowser.selectedTab = gBrowser.tabs[1];
-  testAttrib(gBrowser.tabs[0], "beforeselected-visible", true,
-             "First tab marked beforeselected-visible!");
+  testAttrib(3, "last-visible-tab", true);
+  testAttrib(0, "first-visible-tab", true);
+  testAttrib(2, "beforeselected-visible", false);
+  testAttrib(0, "visuallyselected", true);
+  await switchTab(1);
+  testAttrib(0, "beforeselected-visible", true);
 });
 
 add_task(function cleanup() {
   tabs.forEach(gBrowser.removeTab, gBrowser);
 });
--- a/browser/modules/AsyncTabSwitcher.jsm
+++ b/browser/modules/AsyncTabSwitcher.jsm
@@ -432,16 +432,17 @@ class AsyncTabSwitcher {
         }
       }
 
       // This doesn't necessarily exist if we're a new window and haven't switched tabs yet
       if (this.lastVisibleTab)
         this.lastVisibleTab._visuallySelected = false;
 
       this.visibleTab._visuallySelected = true;
+      this.tabbrowser.tabContainer._setPositionalAttributes();
     }
 
     this.lastVisibleTab = this.visibleTab;
   }
 
   assert(cond) {
     if (!cond) {
       dump("Assertion failure\n" + Error().stack);