Bug 1364669 - Tried to implement mikedeboer's recommendation, still reproducing bug. r?mikedeboer draft
authorJared Wein <jwein@mozilla.com>
Thu, 13 Jul 2017 17:54:08 -0400
changeset 608598 964829339ce2c87778e05c751a1755d7e4f67106
parent 606958 6fec4855b5345eb63fef57089e61829b88f5f4eb
child 637366 d64597727933ff3063b14ce6bb3a485b286bb16d
push id68347
push userbmo:jaws@mozilla.com
push dateThu, 13 Jul 2017 22:15:32 +0000
reviewersmikedeboer
bugs1364669
milestone56.0a1
Bug 1364669 - Tried to implement mikedeboer's recommendation, still reproducing bug. r?mikedeboer @Mike, what do you think about this patch? It doesn't fix it for me but I don't understand why. I've got debugging statements in here and when I follow the STR with the browser console open, what I'm seeing makes sense in the console but the visuals of the browser don't match up. Thoughts??? MozReview-Commit-ID: DvUshZJiNnJ
browser/base/content/tabbrowser.xml
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -5981,16 +5981,17 @@
       </field>
 
       <field name="_firstTab">null</field>
       <field name="_lastTab">null</field>
       <field name="_afterSelectedTab">null</field>
       <field name="_beforeHoveredTab">null</field>
       <field name="_afterHoveredTab">null</field>
       <field name="_hoveredTab">null</field>
+      <field name="_blockHoverChanges">false</field>
       <field name="restoreTabsButton">
         document.getAnonymousElementByAttribute(this, "anonid", "restore-tabs-button");
       </field>
       <field name="_restoreTabsButtonWrapperWidth">0</field>
       <field name="windowUtils">
         window.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils);
       </field>
 
@@ -6107,26 +6108,30 @@
                  (!AppConstants.MOZ_PHOTON_THEME && root.getAttribute("customize-exiting") == "true");
         ]]></getter>
       </property>
 
       <method name="_setPositionalAttributes">
         <body><![CDATA[
           let visibleTabs = this.tabbrowser.visibleTabs;
 
+          console.log(`_blockHoverChanges: ${this._blockHoverChanges}\n_beforeHoveredTab: ${this._beforeHoveredTab}\n${(new Error()).stack}`);
+
           if (!visibleTabs.length)
             return;
 
           let selectedIndex = visibleTabs.indexOf(this.selectedItem);
 
           let lastVisible = visibleTabs.length - 1;
 
           if (this._afterSelectedTab)
             this._afterSelectedTab.removeAttribute("afterselected-visible");
-          if (this.selectedItem.closing || selectedIndex == lastVisible) {
+          if (this.selectedItem.closing ||
+              selectedIndex == lastVisible ||
+              this._blockHoverChanges) {
             this._afterSelectedTab = null;
           } else {
             this._afterSelectedTab = visibleTabs[selectedIndex + 1];
             this._afterSelectedTab.setAttribute("afterselected-visible",
                                                 "true");
           }
 
           if (this._firstTab)
@@ -6933,16 +6938,20 @@
         event.preventDefault();
       ]]></handler>
 
       <handler event="dragstart"><![CDATA[
         var tab = this._getDragTargetTab(event, false);
         if (!tab || this._isCustomizing)
           return;
 
+        if (this._hoveredTab) {
+          this._hoveredTab._mouseleave();
+        }
+
         let dt = event.dataTransfer;
         dt.mozSetDataAt(TAB_DROP_TYPE, tab, 0);
         let browser = tab.linkedBrowser;
 
         // We must not set text/x-moz-url or text/plain data here,
         // otherwise trying to deatch the tab by dropping it on the desktop
         // may result in an "internet shortcut"
         dt.mozSetDataAt("text/x-moz-text-internal", browser.currentURI.spec, 0);
@@ -7160,29 +7169,32 @@
           let dropIndex = "animDropIndex" in draggedTab._dragData &&
                           draggedTab._dragData.animDropIndex;
           if (dropIndex && dropIndex > draggedTab._tPos)
             dropIndex--;
 
           let animate = this.tabbrowser.animationsEnabled;
           if (oldTranslateX && oldTranslateX != newTranslateX && animate) {
             draggedTab.setAttribute("tabdrop-samewindow", "true");
+            let tabContainer = gBrowser.tabContainer;
+            tabContainer._blockHoverChanges = true;
             draggedTab.style.transform = "translateX(" + newTranslateX + "px)";
             let onTransitionEnd = transitionendEvent => {
               if (transitionendEvent.propertyName != "transform" ||
                   transitionendEvent.originalTarget != draggedTab) {
                 return;
               }
               draggedTab.removeEventListener("transitionend", onTransitionEnd);
 
               draggedTab.removeAttribute("tabdrop-samewindow");
 
               this._finishAnimateTabMove();
               if (dropIndex !== false)
                 this.tabbrowser.moveTabTo(draggedTab, dropIndex);
+              tabContainer._blockHoverChanges = false;
             }
             draggedTab.addEventListener("transitionend", onTransitionEnd);
           } else {
             this._finishAnimateTabMove();
             if (dropIndex !== false)
               this.tabbrowser.moveTabTo(draggedTab, dropIndex);
           }
         } else if (draggedTab) {
@@ -7539,20 +7551,23 @@
       While it would make sense to track this in a field, the field will get nuked
       once the node is gone from the DOM, which causes us to think the tab is not
       closed, which causes us to make wrong decisions. So we use an expando instead.
       <field name="closing">false</field>
       -->
 
       <method name="_mouseenter">
         <body><![CDATA[
-          if (this.hidden || this.closing)
+          let tabContainer = this.parentNode;
+          console.log(`_blockHoverChanges: ${tabContainer._blockHoverChanges}\n_beforeHoveredTab:${tabContainer._beforeHoveredTab}\n${(new Error()).stack}`);
+
+          let foo = tabContainer ? tabContainer : null;
+          if (this.hidden || this.closing || tabContainer._blockHoverChanges)
             return;
 
-          let tabContainer = this.parentNode;
           let visibleTabs = tabContainer.tabbrowser.visibleTabs;
           let tabIndex = visibleTabs.indexOf(this);
 
           if (this.selected)
             tabContainer._handleTabSelect();
 
           if (tabIndex == 0) {
             tabContainer._beforeHoveredTab = null;
@@ -7575,16 +7590,17 @@
           }
 
           tabContainer._hoveredTab = this;
         ]]></body>
       </method>
 
       <method name="_mouseleave">
         <body><![CDATA[
+          console.log((new Error()).stack);
           let tabContainer = this.parentNode;
           if (tabContainer._beforeHoveredTab) {
             tabContainer._beforeHoveredTab.removeAttribute("beforehovered");
             tabContainer._beforeHoveredTab = null;
           }
           if (tabContainer._afterHoveredTab) {
             tabContainer._afterHoveredTab.removeAttribute("afterhovered");
             tabContainer._afterHoveredTab = null;