Bug 1371523 - remove find bar sync ipc message, r=mikedeboer
MozReview-Commit-ID: C0VO0U3UJ76
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -49,19 +49,18 @@ window._gBrowser = {
this.selectedBrowser);
}
messageManager.addMessageListener("DOMWindowFocus", this);
messageManager.addMessageListener("RefreshBlocker:Blocked", this);
messageManager.addMessageListener("Browser:WindowCreated", this);
// To correctly handle keypresses for potential FindAsYouType, while
// the tab's find bar is not yet initialized.
- this._findAsYouType = Services.prefs.getBoolPref("accessibility.typeaheadfind");
- Services.prefs.addObserver("accessibility.typeaheadfind", this);
messageManager.addMessageListener("Findbar:Keypress", this);
+ this._setFindbarData();
XPCOMUtils.defineLazyPreferenceGetter(this, "animationsEnabled",
"toolkit.cosmeticAnimations.enabled");
XPCOMUtils.defineLazyPreferenceGetter(this, "schedulePressureDefaultCount",
"browser.schedulePressure.defaultCount");
this._setupEventListeners();
},
@@ -433,16 +432,37 @@ window._gBrowser = {
set userTypedValue(val) {
return this.selectedBrowser.userTypedValue = val;
},
get userTypedValue() {
return this.selectedBrowser.userTypedValue;
},
+ _setFindbarData() {
+ // Ensure we know what the find bar key is in the content process:
+ let initialProcessData = Services.ppmm.initialProcessData;
+ if (!initialProcessData.findBarShortcutData) {
+ let keyEl = document.getElementById("key_find");
+ let mods = keyEl.getAttribute("modifiers")
+ .replace(/accel/i, AppConstants.platform == "macosx" ? "meta" : "control");
+ initialProcessData.findBarShortcutData = {
+ key: keyEl.getAttribute("key"),
+ modifiers: {
+ shiftKey: mods.includes("shift"),
+ ctrlKey: mods.includes("control"),
+ altKey: mods.includes("alt"),
+ metaKey: mods.includes("meta"),
+ },
+ };
+ Services.ppmm.broadcastAsyncMessage("Findbar:ShortcutData",
+ initialProcessData.findBarShortcutData);
+ }
+ },
+
isFindBarInitialized(aTab) {
return (aTab || this.selectedTab)._findBar != undefined;
},
/**
* Get the already constructed findbar
*/
getCachedFindBar(aTab = this.selectedTab) {
@@ -464,37 +484,29 @@ window._gBrowser = {
aTab._pendingFindBar = this._createFindBar(aTab);
}
return aTab._pendingFindBar;
},
/**
* Create a findbar instance.
* @param aTab the tab to create the find bar for.
- * @param aForce Whether to force a sync flush to trigger XBL construction immediately.
* @return the created findbar, or null if the window or tab is closed/closing.
*/
- async _createFindBar(aTab, aForce = false) {
+ async _createFindBar(aTab) {
let findBar = document.createElementNS(this._XUL_NS, "findbar");
let browser = this.getBrowserForTab(aTab);
let browserContainer = this.getBrowserContainer(browser);
browserContainer.appendChild(findBar);
- if (aForce) {
- // Force a style flush to ensure that our binding is attached.
- // Remove after bug 1371523 makes more of this async.
- findBar.clientTop;
- } else {
- await new Promise(r => requestAnimationFrame(r));
- if (window.closed || aTab.closing) {
- delete aTab._pendingFindBar;
- return null;
- }
- }
+ await new Promise(r => requestAnimationFrame(r));
delete aTab._pendingFindBar;
+ if (window.closed || aTab.closing) {
+ return null;
+ }
findBar.browser = browser;
findBar._findField.value = this._lastFindValue;
aTab._findBar = findBar;
let event = document.createEvent("Events");
event.initEvent("TabFindInitialized", true, false);
@@ -3782,40 +3794,21 @@ window._gBrowser = {
updateUserContextUIIndicator();
}
break;
}
case "Findbar:Keypress":
{
let tab = this.getTabForBrowser(browser);
- // If the find bar for this tab is not yet alive, only initialize
- // it if there's a possibility FindAsYouType will be used.
- // There's no point in doing it for most random keypresses.
- if (!this.isFindBarInitialized(tab) &&
- data.shouldFastFind) {
- let shouldFastFind = this._findAsYouType;
- if (!shouldFastFind) {
- // Please keep in sync with toolkit/content/widgets/findbar.xml
- const FAYT_LINKS_KEY = "'";
- const FAYT_TEXT_KEY = "/";
- let charCode = data.fakeEvent.charCode;
- let key = charCode ? String.fromCharCode(charCode) : null;
- shouldFastFind = key == FAYT_LINKS_KEY || key == FAYT_TEXT_KEY;
- }
- if (shouldFastFind) {
- // Make sure we return the result.
- // This needs sync initialization of the find bar, unfortunately.
- // bug 1371523 tracks removing all of this.
-
- // This returns a promise, so don't use the result...
- this._createFindBar(tab, true);
- // ... just grab the 'cached' version now we know it exists.
- this.getCachedFindBar().receiveMessage(aMessage);
- }
+ if (!this.isFindBarInitialized(tab)) {
+ let fakeEvent = data;
+ this.getFindBar(tab).then(findbar => {
+ findbar._onBrowserKeypress(fakeEvent);
+ });
}
break;
}
case "RefreshBlocker:Blocked":
{
// The data object is expected to contain the following properties:
// - URI (string)
// The URI that a page is attempting to refresh or redirect to.
@@ -3874,22 +3867,16 @@ window._gBrowser = {
{
for (let tab of this.tabs) {
if (tab.getAttribute("usercontextid") == aData) {
ContextualIdentityService.setTabStyle(tab);
}
}
break;
}
- case "nsPref:changed":
- {
- // This is the only pref observed.
- this._findAsYouType = Services.prefs.getBoolPref("accessibility.typeaheadfind");
- break;
- }
}
},
_generateUniquePanelID() {
if (!this._uniquePanelIDCounter) {
this._uniquePanelIDCounter = 0;
}
@@ -3940,18 +3927,16 @@ window._gBrowser = {
let messageManager = window.getGroupMessageManager("browsers");
messageManager.removeMessageListener("DOMTitleChanged", this);
window.messageManager.removeMessageListener("contextmenu", this);
if (this._switcher) {
this._switcher.destroy();
}
}
-
- Services.prefs.removeObserver("accessibility.typeaheadfind", this);
},
_setupEventListeners() {
this.addEventListener("DOMWindowClose", (event) => {
if (!event.isTrusted)
return;
if (this.tabs.length == 1) {
--- a/dom/html/test/forms/test_input_time_key_events.html
+++ b/dom/html/test/forms/test_input_time_key_events.html
@@ -167,34 +167,37 @@ var testData = [
{
// Incomplete value maps to empty .value.
keys: ["1"],
initialVal: "",
expectedVal: ""
},
];
-function sendKeys(aKeys) {
+function sendKeys(aKeys, aElem) {
for (let i = 0; i < aKeys.length; i++) {
+ // Force layout flush between keys to ensure focus is correct.
+ // This shouldn't be necessary; bug 1450219 tracks this.
+ aElem.clientTop;
let key = aKeys[i];
if (key.startsWith("KEY_")) {
synthesizeKey(key);
} else {
sendString(key);
}
}
}
function test() {
var elem = document.getElementById("input");
for (let { keys, initialVal, expectedVal } of testData) {
elem.focus();
elem.value = initialVal;
- sendKeys(keys);
+ sendKeys(keys, elem);
is(elem.value, expectedVal,
"Test with " + keys + ", result should be " + expectedVal);
elem.value = "";
elem.blur();
}
}
</script>
--- a/toolkit/content/browser-content.js
+++ b/toolkit/content/browser-content.js
@@ -12,16 +12,18 @@ ChromeUtils.import("resource://gre/modul
ChromeUtils.defineModuleGetter(this, "ReaderMode",
"resource://gre/modules/ReaderMode.jsm");
ChromeUtils.defineModuleGetter(this, "BrowserUtils",
"resource://gre/modules/BrowserUtils.jsm");
ChromeUtils.defineModuleGetter(this, "SelectContentHelper",
"resource://gre/modules/SelectContentHelper.jsm");
ChromeUtils.defineModuleGetter(this, "FindContent",
"resource://gre/modules/FindContent.jsm");
+ChromeUtils.defineModuleGetter(this, "RemoteFinder",
+ "resource://gre/modules/RemoteFinder.jsm");
var global = this;
// Lazily load the finder code
addMessageListener("Finder:Initialize", function() {
let {RemoteFinderListener} = ChromeUtils.import("resource://gre/modules/RemoteFinder.jsm", {});
new RemoteFinderListener(global);
@@ -922,42 +924,93 @@ addMessageListener("SwitchDocumentDirect
var FindBar = {
/* Please keep in sync with toolkit/content/widgets/findbar.xml */
FIND_NORMAL: 0,
FIND_TYPEAHEAD: 1,
FIND_LINKS: 2,
_findMode: 0,
+ /**
+ * _findKey and _findModifiers are used to determine whether a keypress
+ * is a user attempting to use the find shortcut, after which we'll
+ * route keypresses to the parent until we know the findbar has focus
+ * there. To do this, we need shortcut data from the parent.
+ */
+ _findKey: null,
+ _findModifiers: null,
+
init() {
addMessageListener("Findbar:UpdateState", this);
Services.els.addSystemEventListener(global, "keypress", this, false);
Services.els.addSystemEventListener(global, "mouseup", this, false);
+ this._initShortcutData();
},
receiveMessage(msg) {
switch (msg.name) {
case "Findbar:UpdateState":
this._findMode = msg.data.findMode;
+ this._quickFindTimeout = msg.data.hasQuickFindTimeout;
+ if (msg.data.isOpenAndFocused) {
+ this._keepPassingUntilToldOtherwise = false;
+ }
+ break;
+ case "Findbar:ShortcutData":
+ // Set us up to never need this again for the lifetime of this process,
+ // and remove the listener.
+ Services.cpmm.initialProcessData.findBarShortcutData = msg.data;
+ Services.cpmm.removeMessageListener("Findbar:ShortcutData", this);
+ this._initShortcutData(msg.data);
break;
}
},
handleEvent(event) {
switch (event.type) {
case "keypress":
this._onKeypress(event);
break;
case "mouseup":
this._onMouseup(event);
break;
}
},
/**
+ * Use initial process data for find key/modifier data if we have it.
+ * Otherwise, add a listener so we get the data when the parent process has
+ * it.
+ */
+ _initShortcutData(data = Services.cpmm.initialProcessData.findBarShortcutData) {
+ if (data) {
+ this._findKey = data.key;
+ this._findModifiers = data.modifiers;
+ } else {
+ Services.cpmm.addMessageListener("Findbar:ShortcutData", this);
+ }
+ },
+
+ /**
+ * Check whether this key event will start the findbar in the parent,
+ * in which case we should pass any further key events to the parent to avoid
+ * them being lost.
+ * @param aEvent the key event to check.
+ */
+ _eventMatchesFindShortcut(aEvent) {
+ let modifiers = this._findModifiers;
+ if (!modifiers) {
+ return false;
+ }
+ return aEvent.ctrlKey == modifiers.ctrlKey && aEvent.altKey == modifiers.altKey &&
+ aEvent.shiftKey == modifiers.shiftKey && aEvent.metaKey == modifiers.metaKey &&
+ aEvent.key == this._findKey;
+ },
+
+ /**
* Returns whether FAYT can be used for the given event in
* the current content state.
*/
_canAndShouldFastFind() {
let should = false;
let can = BrowserUtils.canFastFind(content);
if (can) {
// XXXgijs: why all these shenanigans? Why not use the event's target?
@@ -965,46 +1018,72 @@ var FindBar = {
let elt = Services.focus.getFocusedElementForWindow(content, true, focusedWindow);
let win = focusedWindow.value;
should = BrowserUtils.shouldFastFind(elt, win);
}
return { can, should };
},
_onKeypress(event) {
+ const FAYT_LINKS_KEY = "'";
+ const FAYT_TEXT_KEY = "/";
+ if (this._eventMatchesFindShortcut(event)) {
+ this._keepPassingUntilToldOtherwise = true;
+ }
// Useless keys:
if (event.ctrlKey || event.altKey || event.metaKey || event.defaultPrevented) {
- return undefined;
+ return;
}
// Check the focused element etc.
let fastFind = this._canAndShouldFastFind();
// Can we even use find in this page at all?
if (!fastFind.can) {
- return undefined;
+ return;
+ }
+ if (this._keepPassingUntilToldOtherwise) {
+ this._passKeyToParent(event);
+ return;
+ }
+ if (!fastFind.should) {
+ return;
}
+ let charCode = event.charCode;
+ // If the find bar is open and quick find is on, send the key to the parent.
+ if (this._findMode != this.FIND_NORMAL && this._quickFindTimeout) {
+ if (!charCode)
+ return;
+ this._passKeyToParent(event);
+ } else {
+ let key = charCode ? String.fromCharCode(charCode) : null;
+ let manualstartFAYT = (key == FAYT_LINKS_KEY || key == FAYT_TEXT_KEY);
+ let autostartFAYT = !manualstartFAYT && RemoteFinder._findAsYouType && key && key != " ";
+ if (manualstartFAYT || autostartFAYT) {
+ let mode = (key == FAYT_LINKS_KEY || (autostartFAYT && RemoteFinder._typeAheadLinksOnly)) ?
+ this.FIND_LINKS : this.FIND_TYPEAHEAD;
+ // Set _findMode immediately (without waiting for child->parent->child roundtrip)
+ // to ensure we pass any further keypresses, too.
+ this._findMode = mode;
+ this._passKeyToParent(event);
+ }
+ }
+ },
+
+ _passKeyToParent(event) {
+ event.preventDefault();
let fakeEvent = {};
for (let k in event) {
if (typeof event[k] != "object" && typeof event[k] != "function" &&
!(k in content.KeyboardEvent)) {
fakeEvent[k] = event[k];
}
}
- // sendSyncMessage returns an array of the responses from all listeners
- let rv = sendSyncMessage("Findbar:Keypress", {
- fakeEvent,
- shouldFastFind: fastFind.should
- });
- if (rv.includes(false)) {
- event.preventDefault();
- return false;
- }
- return undefined;
+ sendAsyncMessage("Findbar:Keypress", fakeEvent);
},
_onMouseup(event) {
if (this._findMode != this.FIND_NORMAL)
sendAsyncMessage("Findbar:Mouseup");
},
};
FindBar.init();
--- a/toolkit/content/tests/browser/browser_findbar.js
+++ b/toolkit/content/tests/browser/browser_findbar.js
@@ -156,47 +156,44 @@ add_task(async function test_reinitializ
BrowserTestUtils.removeTab(tab);
});
/**
* Ensure that the initial typed characters aren't lost immediately after
* opening the find bar.
*/
-add_task(async function() {
+add_task(async function e10sLostKeys() {
// This test only makes sence in e10s evironment.
if (!gMultiProcessBrowser) {
info("Skipping this test because of non-e10s environment.");
return;
}
let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_PAGE_URI);
- let browser = tab.linkedBrowser;
ok(!gFindBarInitialized, "findbar isn't initialized yet");
await gFindBarPromise;
let findBar = gFindBar;
let initialValue = findBar._findField.value;
- await EventUtils.synthesizeAndWaitKey("f", { accelKey: true }, window, null,
- () => {
+ await EventUtils.synthesizeAndWaitKey("f", { accelKey: true }, window, null, () => {
+ // We can't afford to wait for the promise to resolve, by then the
+ // find bar is visible and focused, so sending characters to the
+ // content browser wouldn't work.
isnot(document.activeElement, findBar._findField.inputField,
"findbar is not yet focused");
+ EventUtils.synthesizeKey("a");
+ EventUtils.synthesizeKey("b");
+ EventUtils.synthesizeKey("c");
+ is(findBar._findField.value, initialValue, "still has initial find query");
});
- let promises = [
- BrowserTestUtils.sendChar("a", browser),
- BrowserTestUtils.sendChar("b", browser),
- BrowserTestUtils.sendChar("c", browser)
- ];
-
- is(findBar._findField.value, initialValue, "still has initial find query");
-
- await Promise.all(promises);
+ await BrowserTestUtils.waitForCondition(() => findBar._findField.value.length == 3);
is(document.activeElement, findBar._findField.inputField,
"findbar is now focused");
is(findBar._findField.value, "abc", "abc fully entered as find query");
BrowserTestUtils.removeTab(tab);
});
function promiseFindFinished(searchText, highlightOn) {
--- a/toolkit/content/widgets/findbar.xml
+++ b/toolkit/content/widgets/findbar.xml
@@ -110,29 +110,31 @@
<handler event="blur"><![CDATA[
let findbar = this.findbar;
// Note: This code used to remove the selection
// if it matched an editable.
findbar.browser.finder.enableSelection();
]]></handler>
<handler event="focus"><![CDATA[
+ let findbar = this.findbar;
if (/Mac/.test(navigator.platform)) {
- let findbar = this.findbar;
findbar._onFindFieldFocus();
}
+ findbar._updateBrowserWithState();
]]></handler>
<handler event="compositionstart"><![CDATA[
// Don't close the find toolbar while IME is composing.
let findbar = this.findbar;
findbar._isIMEComposing = true;
if (findbar._quickFindTimeout) {
clearTimeout(findbar._quickFindTimeout);
findbar._quickFindTimeout = null;
+ findbar._updateBrowserWithState();
}
]]></handler>
<handler event="compositionend"><![CDATA[
let findbar = this.findbar;
findbar._isIMEComposing = false;
if (findbar._findMode != findbar.FIND_NORMAL)
findbar._setFindCloseTimeout();
@@ -462,24 +464,26 @@
<body><![CDATA[
if (this._quickFindTimeout)
clearTimeout(this._quickFindTimeout);
// Don't close the find toolbar while IME is composing OR when the
// findbar is already hidden.
if (this._isIMEComposing || this.hidden) {
this._quickFindTimeout = null;
+ this._updateBrowserWithState();
return;
}
this._quickFindTimeout = setTimeout(() => {
if (this._findMode != this.FIND_NORMAL)
this.close();
this._quickFindTimeout = null;
}, this._quickFindTimeoutLength);
+ this._updateBrowserWithState();
]]></body>
</method>
<field name="_pluralForm">null</field>
<property name="pluralForm">
<getter><![CDATA[
if (!this._pluralForm) {
this._pluralForm = ChromeUtils.import(
@@ -741,16 +745,17 @@
this.hidden = true;
// 'focusContent()' iterates over all listeners in the chrome
// process, so we need to call it from here.
this.browser.finder.focusContent();
this.browser.finder.onFindbarClose();
this._cancelTimers();
+ this._updateBrowserWithState();
this._findFailedString = null;
]]></body>
</method>
<method name="clear">
<body><![CDATA[
this.browser.finder.removeSelection();
@@ -825,51 +830,36 @@
return false;
if (this._typeAheadCaseSensitive == 1)
return true;
return aString != aString.toLowerCase();
]]></body>
</method>
- <!-- We get a fake event object through an IPC message which contains the
- data we need to make a decision. We then return |true| if and only if
- the page gets to deal with the event itself. Everywhere we return
- false, the message sender will take care of calling event.preventDefault
- on the real event. -->
+ <!-- We get a fake event object through an IPC message when FAYT is being used
+ from within the browser. We then stuff that input in the find bar here. -->
<method name="_onBrowserKeypress">
<parameter name="aFakeEvent"/>
- <parameter name="aShouldFastFind"/>
<body><![CDATA[
const FAYT_LINKS_KEY = "'";
const FAYT_TEXT_KEY = "/";
- // Fast keypresses can stack up when the content process is slow or
- // hangs when in e10s mode. We make sure the findbar isn't 'opened'
- // several times in a row, because then the find query is selected
- // each time, losing characters typed initially.
- let inputField = this._findField.inputField;
- if (!this.hidden && document.activeElement == inputField) {
- this._dispatchKeypressEvent(inputField, aFakeEvent);
- return false;
+ if (!this.hidden && this._findField.inputField == document.activeElement) {
+ this._dispatchKeypressEvent(this._findField.inputField, aFakeEvent);
+ return;
}
if (this._findMode != this.FIND_NORMAL && this._quickFindTimeout) {
- if (!aFakeEvent.charCode)
- return true;
-
this._findField.select();
this._findField.focus();
this._dispatchKeypressEvent(this._findField.inputField, aFakeEvent);
- return false;
+ return;
}
- if (!aShouldFastFind)
- return true;
-
let key = aFakeEvent.charCode ? String.fromCharCode(aFakeEvent.charCode) : null;
let manualstartFAYT = (key == FAYT_LINKS_KEY || key == FAYT_TEXT_KEY);
let autostartFAYT = !manualstartFAYT && this._findAsYouType &&
key && key != " ";
if (manualstartFAYT || autostartFAYT) {
let mode = (key == FAYT_LINKS_KEY ||
(autostartFAYT && this._typeAheadLinksOnly)) ?
this.FIND_LINKS : this.FIND_TYPEAHEAD;
@@ -882,49 +872,47 @@
this._setFindCloseTimeout();
this._findField.select();
this._findField.focus();
if (autostartFAYT)
this._dispatchKeypressEvent(this._findField.inputField, aFakeEvent);
else
this._updateStatusUI(this.nsITypeAheadFind.FIND_FOUND);
-
- return false;
}
- return undefined;
]]></body>
</method>
<!-- See MessageListener -->
<method name="receiveMessage">
<parameter name="aMessage"/>
<body><![CDATA[
if (aMessage.target != this._browser) {
return undefined;
}
switch (aMessage.name) {
case "Findbar:Mouseup":
if (!this.hidden && this._findMode != this.FIND_NORMAL)
this.close();
break;
-
case "Findbar:Keypress":
- return this._onBrowserKeypress(aMessage.data.fakeEvent,
- aMessage.data.shouldFastFind);
+ this._onBrowserKeypress(aMessage.data);
+ break;
}
return undefined;
]]></body>
</method>
<method name="_updateBrowserWithState">
<body><![CDATA[
if (this._browser && this._browser.messageManager) {
this._browser.messageManager.sendAsyncMessage("Findbar:UpdateState", {
- findMode: this._findMode
+ findMode: this._findMode,
+ isOpenAndFocused: !this.hidden && document.activeElement == this._findField.inputField,
+ hasQuickFindTimeout: !!this._quickFindTimeout,
});
}
]]></body>
</method>
<method name="_enableFindButtons">
<parameter name="aEnable"/>
<body><![CDATA[
--- a/toolkit/modules/RemoteFinder.jsm
+++ b/toolkit/modules/RemoteFinder.jsm
@@ -323,8 +323,14 @@ RemoteFinderListener.prototype = {
break;
case "Finder:ModalHighlightChange":
this._finder.onModalHighlightChange(data.useModalHighlight);
break;
}
}
};
+
+XPCOMUtils.defineLazyPreferenceGetter(RemoteFinder, "_typeAheadLinksOnly",
+ "accessibility.typeaheadfind.linksonly");
+XPCOMUtils.defineLazyPreferenceGetter(RemoteFinder, "_findAsYouType",
+ "accessibility.typeaheadfind");
+