Bug 1303008 - refactor Fennec Finder.jsm integration to always show the correct match count results in the findbar. r?nalexander
MozReview-Commit-ID: KZIstsbHAyT
--- a/mobile/android/base/java/org/mozilla/gecko/FindInPageBar.java
+++ b/mobile/android/base/java/org/mozilla/gecko/FindInPageBar.java
@@ -62,17 +62,19 @@ public class FindInPageBar extends Linea
}
return false;
}
});
mStatusText = (TextView) content.findViewById(R.id.find_status);
mInflated = true;
- GeckoApp.getEventDispatcher().registerGeckoThreadListener(this, "TextSelection:Data");
+ GeckoApp.getEventDispatcher().registerGeckoThreadListener(this,
+ "FindInPage:MatchesCountResult",
+ "TextSelection:Data");
}
public void show() {
if (!mInflated)
inflateContent();
setVisibility(VISIBLE);
mFindText.requestFocus();
@@ -105,17 +107,44 @@ public class FindInPageBar extends Linea
Context context = view.getContext();
return (InputMethodManager) context.getSystemService(Context.INPUT_METHOD_SERVICE);
}
public void onDestroy() {
if (!mInflated) {
return;
}
- GeckoApp.getEventDispatcher().unregisterGeckoThreadListener(this, "TextSelection:Data");
+ GeckoApp.getEventDispatcher().unregisterGeckoThreadListener(this,
+ "FindInPage:MatchesCountResult",
+ "TextSelection:Data");
+ }
+
+ private void onMatchesCountResult(final int total, final int current, final int limit, final String searchString) {
+ if (total == -1) {
+ updateResult(Integer.toString(limit) + "+");
+ } else if (total > 0) {
+ updateResult(Integer.toString(current) + "/" + Integer.toString(total));
+ } else if (TextUtils.isEmpty(searchString)) {
+ updateResult("");
+ } else {
+ // We display 0/0, when there were no
+ // matches found, or if matching has been turned off by setting
+ // pref accessibility.typeaheadfind.matchesCountLimit to 0.
+ updateResult("0/0");
+ }
+ }
+
+ private void updateResult(final String statusText) {
+ ThreadUtils.postToUiThread(new Runnable() {
+ @Override
+ public void run() {
+ mStatusText.setVisibility(statusText.isEmpty() ? View.GONE : View.VISIBLE);
+ mStatusText.setText(statusText);
+ }
+ });
}
// TextWatcher implementation
@Override
public void afterTextChanged(Editable s) {
sendRequestToFinderHelper("FindInPage:Find", s.toString());
}
@@ -155,16 +184,24 @@ public class FindInPageBar extends Linea
hide();
}
}
// GeckoEventListener implementation
@Override
public void handleMessage(String event, JSONObject message) {
+ if (event.equals("FindInPage:MatchesCountResult")) {
+ onMatchesCountResult(message.optInt("total", 0),
+ message.optInt("current", 0),
+ message.optInt("limit", 0),
+ message.optString("searchString"));
+ return;
+ }
+
if (!event.equals("TextSelection:Data") || !REQUEST_ID.equals(message.optString("requestId"))) {
return;
}
final String text = message.optString("text");
// Populate an initial find string, virtual keyboard not required.
if (!TextUtils.isEmpty(text)) {
@@ -198,45 +235,22 @@ public class FindInPageBar extends Linea
/**
* Request find operation, and update matchCount results (current count and total).
*/
private void sendRequestToFinderHelper(final String request, final String searchString) {
GeckoAppShell.sendRequestToGecko(new GeckoRequest(request, searchString) {
@Override
public void onResponse(NativeJSObject nativeJSObject) {
- final int total = nativeJSObject.optInt("total", 0);
- if (total == -1) {
- final int limit = nativeJSObject.optInt("limit", 0);
- updateResult(Integer.toString(limit) + "+");
- } else if (total > 0) {
- final int current = nativeJSObject.optInt("current", 0);
- updateResult(Integer.toString(current) + "/" + Integer.toString(total));
- } else if (TextUtils.isEmpty(searchString)) {
- updateResult("");
- } else {
- // We display 0/0, when there were no
- // matches found, or if matching has been turned off by setting
- // pref accessibility.typeaheadfind.matchesCountLimit to 0.
- updateResult("0/0");
- }
+ // We don't care about the return value, because `onMatchesCountResult`
+ // does the heavy lifting.
}
@Override
public void onError(NativeJSObject error) {
// Gecko didn't respond due to state change, javascript error, etc.
Log.d(LOGTAG, "No response from Gecko on request to match string: [" +
searchString + "]");
updateResult("");
}
-
- private void updateResult(final String statusText) {
- ThreadUtils.postToUiThread(new Runnable() {
- @Override
- public void run() {
- mStatusText.setVisibility(statusText.isEmpty() ? View.GONE : View.VISIBLE);
- mStatusText.setText(statusText);
- }
- });
- }
});
}
}
--- a/mobile/android/chrome/content/FindHelper.js
+++ b/mobile/android/chrome/content/FindHelper.js
@@ -4,17 +4,18 @@
"use strict";
var FindHelper = {
_finder: null,
_targetTab: null,
_initialViewport: null,
_viewportChanged: false,
_result: null,
- _limit: 0,
+
+ // Start of nsIObserver implementation.
observe: function(aMessage, aTopic, aData) {
switch(aTopic) {
case "FindInPage:Opened": {
this._findOpened();
break;
}
@@ -26,44 +27,35 @@ var FindHelper = {
case "FindInPage:Closed":
this._uninit();
this._findClosed();
break;
}
},
+ /**
+ * When the FindInPageBar opens/ becomes visible, it's time to:
+ * 1. Add listeners for other message types sent from the FindInPageBar
+ * 2. initialize the Finder instance, if necessary.
+ */
_findOpened: function() {
- try {
- this._limit = Services.prefs.getIntPref("accessibility.typeaheadfind.matchesCountLimit");
- } catch (e) {
- // Pref not available, assume 0, no match counting.
- this._limit = 0;
- }
-
- Messaging.addListener((data) => {
- this.doFind(data);
- return this._getMatchesCountResult(data);
- }, "FindInPage:Find");
-
- Messaging.addListener((data) => {
- this.findAgain(data, false);
- return this._getMatchesCountResult(data);
- }, "FindInPage:Next");
-
- Messaging.addListener((data) => {
- this.findAgain(data, true);
- return this._getMatchesCountResult(data);
- }, "FindInPage:Prev");
+ Messaging.addListener(data => this.doFind(data), "FindInPage:Find");
+ Messaging.addListener(data => this.findAgain(data, false), "FindInPage:Next");
+ Messaging.addListener(data => this.findAgain(data, true), "FindInPage:Prev");
// Initialize the finder component for the current page by performing a fake find.
this._init();
- this._finder.requestMatchesCount("", 1);
+ this._finder.requestMatchesCount("");
},
+ /**
+ * Fetch the Finder instance from the active tabs' browser and start tracking
+ * the active viewport.
+ */
_init: function() {
// If there's no find in progress, start one.
if (this._finder) {
return;
}
this._targetTab = BrowserApp.selectedTab;
try {
@@ -73,78 +65,124 @@ var FindHelper = {
"JS stack: \n" + (e.stack || Components.stack.formattedStack));
}
this._finder.addResultListener(this);
this._initialViewport = JSON.stringify(this._targetTab.getViewport());
this._viewportChanged = false;
},
+ /**
+ * Detach from the Finder instance (so stop listening for messages) and stop
+ * tracking the active viewport.
+ */
_uninit: function() {
// If there's no find in progress, there's nothing to clean up.
if (!this._finder) {
return;
}
this._finder.removeSelection();
this._finder.removeResultListener(this);
this._finder = null;
this._targetTab = null;
this._initialViewport = null;
this._viewportChanged = false;
},
+ /**
+ * When the FindInPageBar closes, it's time to stop listening for its messages.
+ */
_findClosed: function() {
Messaging.removeListener("FindInPage:Find");
Messaging.removeListener("FindInPage:Next");
Messaging.removeListener("FindInPage:Prev");
},
/**
- * Request, wait for, and return the current matchesCount results for a string.
+ * Start an asynchronous find-in-page operation, using the current Finder
+ * instance and request to count the amount of matches.
+ * If no Finder instance is currently active, we'll lazily initialize it here.
+ *
+ * @param {String} searchString Word to search for in the current document
+ * @return {Object} Echo of the current find action
*/
- _getMatchesCountResult: function(findString) {
- // Count matches up to any provided limit.
- if (this._limit <= 0) {
- return { total: 0, current: 0, limit: 0 };
- }
-
- // Sync call to Finder, results available immediately.
- this._finder.requestMatchesCount(findString, this._limit);
- return this._result;
- },
-
- /**
- * Pass along the count results to FindInPageBar for display.
- */
- onMatchesCountResult: function(result) {
- this._result = result;
- this._result.limit = this._limit;
- },
-
doFind: function(searchString) {
if (!this._finder) {
this._init();
}
this._finder.fastFind(searchString, false);
+ return { searchString, findBackwards: false };
},
+ /**
+ * Restart the same find-in-page operation as before via `doFind()`. If we
+ * haven't called `doFind()`, we simply kick off a regular find.
+ *
+ * @param {String} searchString Word to search for in the current document
+ * @param {Boolean} findBackwards Direction to search in
+ * @return {Object} Echo of the current find action
+ */
findAgain: function(searchString, findBackwards) {
// This always happens if the user taps next/previous after re-opening the
// search bar, and not only forces _init() but also an initial fastFind(STRING)
// before any findAgain(DIRECTION).
if (!this._finder) {
- this.doFind(searchString);
- return;
+ return this.doFind(searchString);
}
this._finder.findAgain(findBackwards, false, false);
+ return { searchString, findBackwards };
+ },
+
+ // Start of Finder.jsm listener implementation.
+
+ /**
+ * Pass along the count results to FindInPageBar for display. The result that
+ * is sent to the FindInPageBar is augmented with the current find-in-page count
+ * limit.
+ *
+ * @param {Object} result Result coming from the Finder instance that contains
+ * the following properties:
+ * - {Number} total The total amount of matches found
+ * - {Number} current The index of current found range
+ * in the document
+ */
+ onMatchesCountResult: function(result) {
+ this._result = result;
+
+ Messaging.sendRequest(Object.assign({
+ type: "FindInPage:MatchesCountResult"
+ }, this._result));
},
+ /**
+ * When a find-in-page action finishes, this method is invoked. This is mainly
+ * used at the moment to detect if the current viewport has changed, which might
+ * be indicated by not finding a string in the current page.
+ *
+ * @param {Object} aData A dictionary, representing the find result, which
+ * contains the following properties:
+ * - {String} searchString Word that was searched for
+ * in the current document
+ * - {Number} result One of the following
+ * Ci.nsITypeAheadFind.* result
+ * indicators: FIND_FOUND,
+ * FIND_NOTFOUND, FIND_WRAPPED,
+ * FIND_PENDING
+ * - {Boolean} findBackwards Whether the search direction
+ * was backwards
+ * - {Boolean} findAgain Whether the previous search
+ * was repeated
+ * - {Boolean} drawOutline Whether we may (re-)draw the
+ * outline of a hyperlink
+ * - {Boolean} linksOnly Whether links-only mode was
+ * active
+ */
onFindResult: function(aData) {
if (aData.result == Ci.nsITypeAheadFind.FIND_NOTFOUND) {
if (this._viewportChanged) {
if (this._targetTab != BrowserApp.selectedTab) {
// this should never happen
Cu.reportError("Warning: selected tab changed during find!");
// fall through and restore viewport on the initial tab anyway
}
--- a/toolkit/content/widgets/findbar.xml
+++ b/toolkit/content/widgets/findbar.xml
@@ -369,18 +369,16 @@
this._foundURL = null;
let prefsvc = this._prefsvc;
this._quickFindTimeoutLength =
prefsvc.getIntPref("accessibility.typeaheadfind.timeout");
this._flashFindBar =
prefsvc.getIntPref("accessibility.typeaheadfind.flashBar");
- this._matchesCountLimit =
- prefsvc.getIntPref("accessibility.typeaheadfind.matchesCountLimit");
this._useModalHighlight = prefsvc.getBoolPref("findbar.modalHighlight");
prefsvc.addObserver("accessibility.typeaheadfind",
this._observer, false);
prefsvc.addObserver("accessibility.typeaheadfind.linksonly",
this._observer, false);
prefsvc.addObserver("accessibility.typeaheadfind.casesensitive",
this._observer, false);
@@ -1304,19 +1302,19 @@
- of the current result.
-->
<method name="onMatchesCountResult">
<parameter name="aResult"/>
<body><![CDATA[
if (aResult.total !== 0) {
if (aResult.total == -1) {
this._foundMatches.value = this.pluralForm.get(
- this._matchesCountLimit,
+ aResult.limit,
this.strBundle.GetStringFromName("FoundMatchesCountLimit")
- ).replace("#1", this._matchesCountLimit);
+ ).replace("#1", aResult.limit);
} else {
this._foundMatches.value = this.pluralForm.get(
aResult.total,
this.strBundle.GetStringFromName("FoundMatches")
).replace("#1", aResult.current)
.replace("#2", aResult.total);
}
this._foundMatches.hidden = false;
--- a/toolkit/modules/Finder.jsm
+++ b/toolkit/modules/Finder.jsm
@@ -389,17 +389,18 @@ Finder.prototype = {
controller.scrollLine(true);
break;
}
},
_notifyMatchesCount: function(result = this._currentMatchesCountResult) {
// The `_currentFound` property is only used for internal bookkeeping.
delete result._currentFound;
- if (result.total == this.matchesCountLimit)
+ result.limit = this.matchesCountLimit;
+ if (result.total == result.limit)
result.total = -1;
for (let l of this._listeners) {
try {
l.onMatchesCountResult(result);
} catch (ex) {}
}