Bug 1362866 - Rearrange tab focusing behaviour to avoid extra potential reflows. r?felipe
MozReview-Commit-ID: F1S179A1GH6
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -1241,36 +1241,20 @@
if (prompts.length) {
// NB: This code assumes that the beforeunload prompt
// is the top-most prompt on the tab.
prompts[prompts.length - 1].abortPrompt();
}
});
}
- oldBrowser._urlbarFocused = (gURLBar && gURLBar.focused);
- if (this.isFindBarInitialized(oldTab)) {
- let findBar = this.getFindBar(oldTab);
- oldTab._findBarFocused = (!findBar.hidden &&
- findBar._findField.getAttribute("focused") == "true");
+ if (!gMultiProcessBrowser) {
+ this._adjustFocusBeforeTabSwitch(oldTab, this.mCurrentTab);
+ this._adjustFocusAfterTabSwitch(this.mCurrentTab);
}
-
- // If focus is in the tab bar, retain it there.
- if (document.activeElement == oldTab) {
- // We need to explicitly focus the new tab, because
- // tabbox.xml does this only in some cases.
- this.mCurrentTab.focus();
- } else if (gMultiProcessBrowser && document.activeElement !== newBrowser) {
- // Clear focus so that _adjustFocusAfterTabSwitch can detect if
- // some element has been focused and respect that.
- document.activeElement.blur();
- }
-
- if (!gMultiProcessBrowser)
- this._adjustFocusAfterTabSwitch(this.mCurrentTab);
}
updateUserContextUIIndicator();
gIdentityHandler.updateSharingIndicator();
this.tabContainer._setPositionalAttributes();
if (!gMultiProcessBrowser) {
@@ -1284,16 +1268,55 @@
}
if (!aForceUpdate)
TelemetryStopwatch.finish("FX_TAB_SWITCH_UPDATE_MS");
]]>
</body>
</method>
+ <method name="_adjustFocusBeforeTabSwitch">
+ <parameter name="oldTab"/>
+ <parameter name="newTab"/>
+ <body><![CDATA[
+ if (this._previewMode) {
+ return;
+ }
+
+ let oldBrowser = oldTab.linkedBrowser;
+ let newBrowser = newTab.linkedBrowser;
+
+ oldBrowser._urlbarFocused = (gURLBar && gURLBar.focused);
+
+ if (this.isFindBarInitialized(oldTab)) {
+ let findBar = this.getFindBar(oldTab);
+ oldTab._findBarFocused = (!findBar.hidden &&
+ findBar._findField.getAttribute("focused") == "true");
+ }
+
+ // If focus is in the tab bar, retain it there.
+ if (document.activeElement == oldTab) {
+ // We need to explicitly focus the new tab, because
+ // tabbox.xml does this only in some cases.
+ newTab.focus();
+ } else if (gMultiProcessBrowser && document.activeElement !== newBrowser) {
+
+ let keepFocusOnUrlBar = newBrowser &&
+ newBrowser._urlbarFocused &&
+ gURLBar &&
+ gURLBar.focused;
+ if (!keepFocusOnUrlBar) {
+ // Clear focus so that _adjustFocusAfterTabSwitch can detect if
+ // some element has been focused and respect that.
+ document.activeElement.blur();
+ }
+ }
+ ]]></body>
+ </method>
+
<method name="_adjustFocusAfterTabSwitch">
<parameter name="newTab"/>
<body><![CDATA[
// Don't steal focus from the tab bar.
if (document.activeElement == newTab)
return;
let newBrowser = this.getBrowserForTab(newTab);
@@ -1309,22 +1332,25 @@
// Focus the location bar if it was previously focused for that tab.
// In full screen mode, only bother making the location bar visible
// if the tab is a blank one.
if (newBrowser._urlbarFocused && gURLBar) {
// Explicitly close the popup if the URL bar retains focus
gURLBar.closePopup();
- if (!window.fullScreen) {
- gURLBar.focus();
+ // If the user happened to type into the URL bar for this browser
+ // by the time we got here, focusing will cause the text to be
+ // selected which could cause them to overwrite what they've
+ // already typed in.
+ if (gURLBar.focused && newBrowser.userTypedValue) {
return;
}
- if (isTabEmpty(this.mCurrentTab)) {
+ if (!window.fullScreen || isTabEmpty(this.mCurrentTab)) {
focusAndSelectUrlBar();
return;
}
}
// Focus the find bar if it was previously focused for that tab.
if (gFindBarInitialized && !gFindBar.hidden &&
this.selectedTab._findBarFocused) {
@@ -1526,16 +1552,17 @@
var aNoReferrer;
var aUserContextId;
var aSameProcessAsFrameLoader;
var aOriginPrincipal;
var aOpener;
var aIsPrerendered;
var aCreateLazyBrowser;
var aNextTabParentId;
+ var aFocusUrlBar;
if (arguments.length == 2 &&
typeof arguments[1] == "object" &&
!(arguments[1] instanceof Ci.nsIURI)) {
let params = arguments[1];
aTriggeringPrincipal = params.triggeringPrincipal
aReferrerURI = params.referrerURI;
aReferrerPolicy = params.referrerPolicy;
aCharset = params.charset;
@@ -1551,16 +1578,17 @@
aNoReferrer = params.noReferrer;
aUserContextId = params.userContextId;
aSameProcessAsFrameLoader = params.sameProcessAsFrameLoader;
aOriginPrincipal = params.originPrincipal;
aOpener = params.opener;
aIsPrerendered = params.isPrerendered;
aCreateLazyBrowser = params.createLazyBrowser;
aNextTabParentId = params.nextTabParentId;
+ aFocusUrlBar = params.focusUrlBar;
}
var bgLoad = (aLoadInBackground != null) ? aLoadInBackground :
Services.prefs.getBoolPref("browser.tabs.loadInBackground");
var owner = bgLoad ? null : this.selectedTab;
var tab = this.addTab(aURI, {
triggeringPrincipal: aTriggeringPrincipal,
@@ -1578,17 +1606,18 @@
createLazyBrowser: aCreateLazyBrowser,
preferredRemoteType: aPreferredRemoteType,
noReferrer: aNoReferrer,
userContextId: aUserContextId,
originPrincipal: aOriginPrincipal,
sameProcessAsFrameLoader: aSameProcessAsFrameLoader,
opener: aOpener,
isPrerendered: aIsPrerendered,
- nextTabParentId: aNextTabParentId });
+ nextTabParentId: aNextTabParentId,
+ focusUrlBar: aFocusUrlBar });
if (!bgLoad)
this.selectedTab = tab;
return tab;
]]>
</body>
</method>
@@ -2283,16 +2312,17 @@
var aOriginPrincipal;
var aDisallowInheritPrincipal;
var aOpener;
var aIsPrerendered;
var aCreateLazyBrowser;
var aSkipBackgroundNotify;
var aNextTabParentId;
var aNoInitialLabel;
+ var aFocusUrlBar;
if (arguments.length == 2 &&
typeof arguments[1] == "object" &&
!(arguments[1] instanceof Ci.nsIURI)) {
let params = arguments[1];
aTriggeringPrincipal = params.triggeringPrincipal;
aReferrerURI = params.referrerURI;
aReferrerPolicy = params.referrerPolicy;
aCharset = params.charset;
@@ -2312,16 +2342,17 @@
aOriginPrincipal = params.originPrincipal;
aDisallowInheritPrincipal = params.disallowInheritPrincipal;
aOpener = params.opener;
aIsPrerendered = params.isPrerendered;
aCreateLazyBrowser = params.createLazyBrowser;
aSkipBackgroundNotify = params.skipBackgroundNotify;
aNextTabParentId = params.nextTabParentId;
aNoInitialLabel = params.noInitialLabel;
+ aFocusUrlBar = params.focusUrlBar;
}
// if we're adding tabs, we're past interrupt mode, ditch the owner
if (this.mCurrentTab.owner)
this.mCurrentTab.owner = null;
var t = document.createElementNS(NS_XUL, "tab");
@@ -2435,16 +2466,21 @@
userContextId: aUserContextId,
sameProcessAsFrameLoader: aSameProcessAsFrameLoader,
opener: aOpener,
isPrerendered: aIsPrerendered,
nextTabParentId: aNextTabParentId });
}
t.linkedBrowser = b;
+
+ if (aFocusUrlBar) {
+ b._urlbarFocused = true;
+ }
+
this._tabForBrowser.set(b, t);
t.permanentKey = b.permanentKey;
t._browserParams = { uriIsAboutBlank,
remoteType,
usingPreloadedContent };
// If the caller opts in, create a lazy browser.
if (aCreateLazyBrowser) {
@@ -4027,18 +4063,16 @@
this.assert(this.lastVisibleTab === this.requestedTab);
this.assert(this.minimized || this.getTabState(this.requestedTab) == this.STATE_LOADED);
this.destroy();
let toBrowser = this.requestedTab.linkedBrowser;
toBrowser.setAttribute("primary", "true");
- this.tabbrowser._adjustFocusAfterTabSwitch(this.requestedTab);
-
let fromBrowser = this.originalTab.linkedBrowser;
// It's possible that the tab we're switching from closed
// before we were able to finalize, in which case, fromBrowser
// doesn't exist.
if (fromBrowser) {
fromBrowser.removeAttribute("primary");
}
@@ -4120,16 +4154,17 @@
}
this.spinnerTab = showTab;
this.tabbrowser.setAttribute("pendingpaint", "true");
this.spinnerTab.linkedBrowser.setAttribute("pendingpaint", "true");
}
// Switch to the tab we've decided to make visible.
if (this.visibleTab !== showTab) {
+ this.tabbrowser._adjustFocusBeforeTabSwitch(this.visibleTab, showTab);
this.visibleTab = showTab;
this.maybeVisibleTabs.add(showTab);
let tabs = this.tabbrowser.mTabBox.tabs;
let tabPanel = this.tabbrowser.mPanelContainer;
let showPanel = tabs.getRelatedElement(showTab);
let index = Array.indexOf(tabPanel.childNodes, showPanel);
--- a/browser/base/content/test/performance/browser_tabopen_reflows.js
+++ b/browser/base/content/test/performance/browser_tabopen_reflows.js
@@ -13,64 +13,34 @@
* for tips on how to do that.
*/
const EXPECTED_REFLOWS = [
// selection change notification may cause querying the focused editor content
// by IME and that will cause reflow.
[
"select@chrome://global/content/bindings/textbox.xml",
"focusAndSelectUrlBar@chrome://browser/content/browser.js",
- "openLinkIn@chrome://browser/content/utilityOverlay.js",
- "openUILinkIn@chrome://browser/content/utilityOverlay.js",
- "BrowserOpenTab@chrome://browser/content/browser.js",
+ "_adjustFocusAfterTabSwitch@chrome://browser/content/tabbrowser.xml",
],
// selection change notification may cause querying the focused editor content
// by IME and that will cause reflow.
[
"select@chrome://global/content/bindings/textbox.xml",
"focusAndSelectUrlBar@chrome://browser/content/browser.js",
- "openLinkIn@chrome://browser/content/utilityOverlay.js",
- "openUILinkIn@chrome://browser/content/utilityOverlay.js",
- "BrowserOpenTab@chrome://browser/content/browser.js",
+ "_adjustFocusAfterTabSwitch@chrome://browser/content/tabbrowser.xml",
],
[
"select@chrome://global/content/bindings/textbox.xml",
"focusAndSelectUrlBar@chrome://browser/content/browser.js",
- "openLinkIn@chrome://browser/content/utilityOverlay.js",
- "openUILinkIn@chrome://browser/content/utilityOverlay.js",
- "BrowserOpenTab@chrome://browser/content/browser.js",
- ],
-
- [
- "openLinkIn@chrome://browser/content/utilityOverlay.js",
- "openUILinkIn@chrome://browser/content/utilityOverlay.js",
- "BrowserOpenTab@chrome://browser/content/browser.js",
- ],
-
- [
- // switching focus in updateCurrentBrowser() causes reflows
"_adjustFocusAfterTabSwitch@chrome://browser/content/tabbrowser.xml",
- "updateDisplay@chrome://browser/content/tabbrowser.xml",
- "postActions@chrome://browser/content/tabbrowser.xml",
- "requestTab@chrome://browser/content/tabbrowser.xml"
],
];
-if (!gMultiProcessBrowser) {
- EXPECTED_REFLOWS.push(
- [
- "_adjustFocusAfterTabSwitch@chrome://browser/content/tabbrowser.xml",
- "updateCurrentBrowser@chrome://browser/content/tabbrowser.xml",
- "onselect@chrome://browser/content/browser.xul",
- ],
- );
-}
-
/*
* This test ensures that there are no unexpected
* uninterruptible reflows when opening new tabs.
*/
add_task(async function() {
// If we've got a preloaded browser, get rid of it so that it
// doesn't interfere with the test if it's loading. We have to
// do this before we disable preloading or changing the new tab
--- a/browser/base/content/utilityOverlay.js
+++ b/browser/base/content/utilityOverlay.js
@@ -391,16 +391,18 @@ function openLinkIn(url, where, params)
// 'where' is "tab" or "tabshifted", so we'll load the link in a new tab.
loadInBackground = aInBackground;
if (loadInBackground == null) {
loadInBackground =
aFromChrome ? false : getBoolPref("browser.tabs.loadInBackground");
}
}
+ let focusUrlBar = false;
+
switch (where) {
case "current":
let flags = Ci.nsIWebNavigation.LOAD_FLAGS_NONE;
if (aAllowThirdPartyFixup) {
flags |= Ci.nsIWebNavigation.LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP;
flags |= Ci.nsIWebNavigation.LOAD_FLAGS_FIXUP_SCHEME_TYPOS;
}
@@ -432,30 +434,33 @@ function openLinkIn(url, where, params)
postData: aPostData,
userContextId: aUserContextId
});
break;
case "tabshifted":
loadInBackground = !loadInBackground;
// fall through
case "tab":
+ focusUrlBar = !loadInBackground && w.isBlankPageURL(url);
+
let tabUsedForLoad = w.gBrowser.loadOneTab(url, {
referrerURI: aReferrerURI,
referrerPolicy: aReferrerPolicy,
charset: aCharset,
postData: aPostData,
inBackground: loadInBackground,
allowThirdPartyFixup: aAllowThirdPartyFixup,
relatedToCurrent: aRelatedToCurrent,
skipAnimation: aSkipTabAnimation,
allowMixedContent: aAllowMixedContent,
noReferrer: aNoReferrer,
userContextId: aUserContextId,
originPrincipal: aPrincipal,
triggeringPrincipal: aTriggeringPrincipal,
+ focusUrlBar,
});
targetBrowser = tabUsedForLoad.linkedBrowser;
if (params.frameOuterWindowID != undefined && w) {
// Only notify it as a WebExtensions' webNavigation.onCreatedNavigationTarget
// event if it contains the expected frameOuterWindowID params.
// (e.g. we should not notify it as a onCreatedNavigationTarget if the user is
// opening a new tab using the keyboard shortcut).
@@ -466,24 +471,20 @@ function openLinkIn(url, where, params)
sourceTabBrowser: w.gBrowser.selectedBrowser,
sourceFrameOuterWindowID: params.frameOuterWindowID,
},
}, "webNavigation-createdNavigationTarget");
}
break;
}
- // Focus the content, but only if the browser used for the load is selected.
- if (targetBrowser == w.gBrowser.selectedBrowser) {
+ if (!focusUrlBar && targetBrowser == w.gBrowser.selectedBrowser) {
+ // Focus the content, but only if the browser used for the load is selected.
targetBrowser.focus();
}
-
- if (!loadInBackground && w.isBlankPageURL(url)) {
- w.focusAndSelectUrlBar();
- }
}
// Used as an onclick handler for UI elements with link-like behavior.
// e.g. onclick="checkForMiddleClick(this, event);"
function checkForMiddleClick(node, event) {
// We should be using the disabled property here instead of the attribute,
// but some elements that this function is used with don't support it (e.g.
// menuitem).
--- a/browser/components/customizableui/test/browser_editcontrols_update.js
+++ b/browser/components/customizableui/test/browser_editcontrols_update.js
@@ -96,16 +96,21 @@ add_task(async function test_panelui_cus
await startCustomizing();
let navbar = document.getElementById("nav-bar").customizationTarget;
simulateItemDrag(document.getElementById("edit-controls"), navbar);
await endCustomizing();
// updateEditUIVisibility should be called when customization ends but isn't. See bug 1359790.
updateEditUIVisibility();
+ // The URL bar may have been focused to begin with, which means
+ // that subsequent calls to focus it won't result in command
+ // updates, so we'll make sure to blur it.
+ gURLBar.blur();
+
let overridePromise = expectCommandUpdate(1);
gURLBar.select();
gURLBar.focus();
gURLBar.value = "other";
await overridePromise;
checkState(false, "Update when edit-controls on toolbar and focused");
overridePromise = expectCommandUpdate(1);