commit 8c9d8d5088bee284901ab83f4c19a558f82944e2
parent a2b0be99c0b0fa271c4521dc2f136cdb1d397290
Author: Nick Grato <ngrato@gmail.com>
Date: Wed, 1 Oct 2025 20:22:49 +0000
Bug 1987972 - Expand context of target word in find flow appox 40 characters before and after r=Mardak,firefox-ai-ml-reviewers
To support the new find flow in the side bar we need to update the the Find system files to return the before and after context of the target word. for example in a wiki page is the target word might be bear and we would want to adjust the range to return something like this.
... the Southern Hemisphere. Bears are found on the continents of North America, South America...
Differential Revision: https://phabricator.services.mozilla.com/D264877
Diffstat:
9 files changed, 434 insertions(+), 46 deletions(-)
diff --git a/browser/components/genai/content/page-assist.css b/browser/components/genai/content/page-assist.css
@@ -6,11 +6,10 @@
padding: var(--space-large);
}
-.prompt-textarea {
- columns: 3;
- width: 100%;
-}
-
.ai-response {
white-space: pre-wrap;
}
+
+#input.find-input {
+ --input-background-icon: url(chrome://global/skin/icons/search-glass.svg);
+}
diff --git a/browser/components/genai/content/page-assist.mjs b/browser/components/genai/content/page-assist.mjs
@@ -2,7 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-import { html } from "chrome://global/content/vendor/lit.all.mjs";
+import { html, ifDefined } from "chrome://global/content/vendor/lit.all.mjs";
import { MozLitElement } from "chrome://global/content/lit-utils.mjs";
// eslint-disable-next-line import/no-unassigned-import
@@ -14,6 +14,37 @@ ChromeUtils.defineESModuleGetters(lazy, {
AboutReaderParent: "resource:///actors/AboutReaderParent.sys.mjs",
});
+import MozInputText from "chrome://global/content/elements/moz-input-text.mjs";
+
+export class PageAssistInput extends MozInputText {
+ static properties = {
+ class: { type: String, reflect: true },
+ };
+
+ inputTemplate() {
+ return html`
+ <link
+ rel="stylesheet"
+ href="chrome://browser/content/genai/content/page-assist.css"
+ />
+ <input
+ id="input"
+ class=${"with-icon " + ifDefined(this.class)}
+ name=${this.name}
+ .value=${this.value || ""}
+ ?disabled=${this.disabled || this.parentDisabled}
+ accesskey=${ifDefined(this.accessKey)}
+ placeholder=${ifDefined(this.placeholder)}
+ aria-label=${ifDefined(this.ariaLabel ?? undefined)}
+ aria-describedby="description"
+ @input=${this.handleInput}
+ @change=${this.redispatchEvent}
+ />
+ `;
+ }
+}
+customElements.define("page-assists-input", PageAssistInput);
+
export class PageAssist extends MozLitElement {
_progressListener = null;
_onTabSelect = null;
@@ -24,6 +55,10 @@ export class PageAssist extends MozLitElement {
userPrompt: { type: String },
aiResponse: { type: String },
isCurrentPageReaderable: { type: Boolean },
+ matchCountQty: { type: Number },
+ currentMatchIndex: { type: Number },
+ highlightAll: { type: Boolean },
+ snippets: { type: Array },
};
constructor() {
@@ -31,6 +66,10 @@ export class PageAssist extends MozLitElement {
this.userPrompt = "";
this.aiResponse = "";
this.isCurrentPageReaderable = true;
+ this.matchCountQty = 0;
+ this.currentMatchIndex = 0;
+ this.highlightAll = true;
+ this.snippets = [];
}
get _browserWin() {
@@ -45,10 +84,16 @@ export class PageAssist extends MozLitElement {
this._attachReaderModeListener();
this._initURLChange();
this._onUnload = () => this._cleanup();
+ this._setupFinder();
this.ownerGlobal.addEventListener("unload", this._onUnload, { once: true });
}
disconnectedCallback() {
+ // Clean up finder listener
+ if (this.browser && this.browser.finder) {
+ this.browser.finder.removeResultListener(this);
+ }
+
if (this._onUnload) {
this.ownerGlobal.removeEventListener("unload", this._onUnload);
this._onUnload = null;
@@ -57,6 +102,35 @@ export class PageAssist extends MozLitElement {
super.disconnectedCallback();
}
+ _setupFinder() {
+ const gBrowser = this._gBrowser;
+
+ if (!gBrowser) {
+ console.warn("No gBrowser found.");
+ return;
+ }
+
+ const selected = gBrowser.selectedBrowser;
+
+ // If already attached to this browser, skip
+ if (this.browser === selected) {
+ return;
+ }
+
+ // Clean up old listener if needed
+ if (this.browser && this.browser.finder) {
+ this.browser.finder.removeResultListener(this);
+ }
+
+ this.browser = selected;
+
+ if (this.browser && this.browser.finder) {
+ this.browser.finder.addResultListener(this);
+ } else {
+ console.warn("PageAssist: no finder on selected browser.");
+ }
+ }
+
_cleanup() {
try {
const gBrowser = this._gBrowser;
@@ -114,6 +188,7 @@ export class PageAssist extends MozLitElement {
}
this._onTabSelect = () => {
+ this._setupFinder();
const browser = gBrowser.selectedBrowser;
this.isCurrentPageReaderable = !!browser?.isArticle;
};
@@ -161,11 +236,66 @@ export class PageAssist extends MozLitElement {
return await actor.fetchPageData();
}
+ _clearFinder() {
+ if (this.browser?.finder) {
+ this.browser.finder.removeSelection();
+ this.browser.finder.highlight(false, "", false);
+ }
+ this.matchCountQty = 0;
+ this.currentMatchIndex = 0;
+ this.snippets = [];
+ }
+
_handlePromptInput = e => {
const value = e.target.value;
this.userPrompt = value;
+
+ // If input is empty, clear values
+ if (!value) {
+ this._clearFinder();
+ return;
+ }
+
+ // Perform the search
+ this.browser.finder.fastFind(value, false, false);
+
+ if (this.highlightAll) {
+ // Todo this also needs to take contextRange.
+ this.browser.finder.highlight(true, value, false);
+ }
+
+ // Request match count - this method will trigger onMatchesCountResult callback
+ this.browser.finder.requestMatchesCount(value, {
+ linksOnly: false,
+ contextRange: 30,
+ });
};
+ onMatchesCountResult(result) {
+ this.matchCountQty = result.total;
+ this.currentMatchIndex = result.current;
+ this.snippets = result.snippets || [];
+ }
+
+ // Abstract method need to be implemented or it will error
+ onHighlightFinished() {
+ // Noop.
+ }
+
+ // Finder result listener methods
+ onFindResult(result) {
+ switch (result.result) {
+ case Ci.nsITypeAheadFind.FIND_NOTFOUND:
+ this.matchCountQty = 0;
+ this.currentMatchIndex = 0;
+ this.snippets = [];
+ break;
+
+ default:
+ break;
+ }
+ }
+
_handleSubmit = async () => {
const pageData = await this._fetchPageData();
if (!pageData) {
@@ -196,10 +326,13 @@ export class PageAssist extends MozLitElement {
? html`<div class="ai-response">${this.aiResponse}</div>`
: ""}
<div>
- <textarea
- class="prompt-textarea"
- @input=${e => this._handlePromptInput(e)}
- ></textarea>
+ <page-assists-input
+ class="find-input"
+ type="text"
+ placeholder="Find in page..."
+ .value=${this.userPrompt}
+ @input=${this._handlePromptInput}
+ ></page-assists-input>
<moz-button
id="submit-user-prompt-btn"
type="primary"
@@ -209,6 +342,22 @@ export class PageAssist extends MozLitElement {
Submit
</moz-button>
</div>
+
+ <div>
+ ${this.snippets.length
+ ? html`<div class="snippets">
+ <h3>Snippets</h3>
+ <ul>
+ ${this.snippets.map(
+ snippet =>
+ html`<li>
+ ${snippet.before}<b>${snippet.match}</b>${snippet.after}
+ </li>`
+ )}
+ </ul>
+ </div>`
+ : ""}
+ </div>
</div>
</div>
`;
diff --git a/browser/components/genai/tests/browser/browser_page_assist_actors.js b/browser/components/genai/tests/browser/browser_page_assist_actors.js
@@ -42,9 +42,11 @@ function assertCommonPageData(pageData) {
);
}
-registerCleanupFunction(() => {
- Services.prefs.clearUserPref("browser.ml.pageAssist.enabled");
+registerCleanupFunction(async () => {
// Ensure sidebar is hidden after each test:
+ try {
+ await SpecialPowers.popPrefEnv(); // undo any pushPrefEnv from this test
+ } catch {}
if (!document.getElementById("sidebar-box").hidden) {
info(
`Sidebar ${SidebarController.currentID} was left open, closing it in cleanup function`
@@ -130,8 +132,8 @@ add_task(async function test_page_assist_sidebar_integration() {
// 2) Grab controls and assert initial enabled state.
const shadowRoot = sidebarEl.shadowRoot;
- const textarea = shadowRoot.querySelector(".prompt-textarea");
- const submitBtn = shadowRoot.querySelector("#submit-user-prompt-btn");
+ const textarea = shadowRoot.querySelector("page-assists-input");
+ const submitBtn = shadowRoot.querySelector("page-assists-input");
Assert.ok(textarea, "Prompt textarea should exist");
Assert.ok(submitBtn, "Submit button should exist");
diff --git a/toolkit/actors/FinderChild.sys.mjs b/toolkit/actors/FinderChild.sys.mjs
@@ -103,11 +103,11 @@ export class FinderChild extends JSWindowActorChild {
case "Finder:MatchesCount":
return this.finder
- .requestMatchesCount(
- data.searchString,
- data.linksOnly,
- data.useSubFrames
- )
+ .requestMatchesCount(data.searchString, {
+ linksOnly: data.linksOnly,
+ useSubFrames: data.useSubFrames,
+ contextRange: data.contextRange,
+ })
.then(result => {
if (result) {
result.browsingContextId = this.browsingContext.id;
diff --git a/toolkit/content/widgets/findbar.js b/toolkit/content/widgets/findbar.js
@@ -500,10 +500,9 @@
return;
}
- this.browser.finder.requestMatchesCount(
- this._findField.value,
- this.findMode == this.FIND_LINKS
- );
+ this.browser.finder.requestMatchesCount(this._findField.value, {
+ linksOnly: this.findMode == this.FIND_LINKS,
+ });
}
/**
diff --git a/toolkit/modules/Finder.sys.mjs b/toolkit/modules/Finder.sys.mjs
@@ -372,11 +372,11 @@ Finder.prototype = {
aArgs,
aArgs.useSubFrames ? false : aArgs.foundInThisFrame
);
- let matchCountPromise = this.requestMatchesCount(
- aArgs.searchString,
- aArgs.linksOnly,
- aArgs.useSubFrames
- );
+
+ let matchCountPromise = this.requestMatchesCount(aArgs.searchString, {
+ linksOnly: aArgs.linksOnly,
+ useSubFrames: aArgs.useSubFrames,
+ });
let results = await Promise.all([highlightPromise, matchCountPromise]);
@@ -606,7 +606,7 @@ Finder.prototype = {
return result;
},
- async requestMatchesCount(aWord, aLinksOnly, aUseSubFrames = true) {
+ async requestMatchesCount(aWord, optionalArgs) {
if (
this._lastFindResult == Ci.nsITypeAheadFind.FIND_NOTFOUND ||
this.searchString == "" ||
@@ -624,11 +624,15 @@ Finder.prototype = {
let params = {
caseSensitive: this._fastFind.caseSensitive,
entireWord: this._fastFind.entireWord,
- linksOnly: aLinksOnly,
+ linksOnly: optionalArgs.linksOnly || false,
matchDiacritics: this._fastFind.matchDiacritics,
word: aWord,
- useSubFrames: aUseSubFrames,
+ useSubFrames:
+ optionalArgs.useSubFrames !== undefined
+ ? optionalArgs.useSubFrames
+ : true,
};
+
if (!this.iterator.continueRunning(params)) {
this.iterator.stop();
}
@@ -639,7 +643,11 @@ Finder.prototype = {
limit: this.matchesCountLimit,
listener: this,
useCache: true,
- useSubFrames: aUseSubFrames,
+ useSubFrames:
+ optionalArgs.useSubFrames !== undefined
+ ? optionalArgs.useSubFrames
+ : true,
+ contextRange: optionalArgs.contextRange || 0,
})
);
@@ -654,13 +662,22 @@ Finder.prototype = {
// FinderIterator listener implementation
- onIteratorRangeFound(range) {
+ onIteratorRangeFound(range, extra) {
let result = this._currentMatchesCountResult;
if (!result) {
return;
}
++result.total;
+
+ // Pull out the snippet that finderIterator attached
+ if (extra?.context) {
+ if (!result.snippets) {
+ result.snippets = [];
+ }
+ result.snippets.push(extra.context);
+ }
+
if (!result._currentFound) {
++result.current;
result._currentFound =
@@ -675,7 +692,10 @@ Finder.prototype = {
onIteratorReset() {},
onIteratorRestart({ word, linksOnly, useSubFrames }) {
- this.requestMatchesCount(word, linksOnly, useSubFrames);
+ this.requestMatchesCount(word, {
+ linksOnly,
+ useSubFrames,
+ });
},
onIteratorStart() {
diff --git a/toolkit/modules/FinderIterator.sys.mjs b/toolkit/modules/FinderIterator.sys.mjs
@@ -78,7 +78,8 @@ export class FinderIterator {
* Optional, defaults to `false`.
* @param {Object} options.listener Listener object that implements the
* following callback functions:
- * - onIteratorRangeFound({Range} range);
+ * - onIteratorRangeFound({Range} range, {Object} extra);
+ * extra.context contains text snippet around the match
* - onIteratorReset();
* - onIteratorRestart({Object} iterParams);
* - onIteratorStart({Object} iterParams);
@@ -90,6 +91,7 @@ export class FinderIterator {
* @param {Boolean} [options.useSubFrames] Whether to iterate over subframes.
* Optional, defaults to `false`.
* @param {String} options.word Word to search for
+ * @param {Number} contextRange - Number of characters to extract before and after target string
* @return {Promise}
*/
start({
@@ -104,6 +106,7 @@ export class FinderIterator {
useCache,
word,
useSubFrames,
+ contextRange,
}) {
// Take care of default values for non-required options.
if (typeof allowDistance != "number") {
@@ -199,7 +202,7 @@ export class FinderIterator {
// Start!
this.running = true;
this._currentParams = iterParams;
- this._findAllRanges(finder, ++this._spawnId);
+ this._findAllRanges(finder, ++this._spawnId, contextRange);
return promise;
}
@@ -370,6 +373,84 @@ export class FinderIterator {
}
/**
+ * Extracts context around a found Range.
+ * Returns normalized text slices so indices are stable with FindBar behavior.
+ *
+ * @param {Range} range - DOM range for a single match.
+ * @param {Number} contextRange - Number of characters to extract before and after target string
+ */
+ _extractSnippet(range, contextRange) {
+ const blockSelector =
+ "p,li,dt,dd,td,th,pre,code,h1,h2,h3,h4,h5,h6,article,section,blockquote,div";
+
+ const defaultResult = {
+ before: "",
+ match: "",
+ after: "",
+ start: -1,
+ end: -1,
+ };
+
+ try {
+ const doc = range?.startContainer?.ownerDocument;
+ if (!doc || range.collapsed) {
+ return defaultResult;
+ }
+
+ // Pick a stable block container for context
+ let node = range.commonAncestorContainer;
+ if (node.nodeType === Node.TEXT_NODE) {
+ node = node.parentElement || node.parentNode;
+ }
+
+ // Find the nearest block-level container (p, div, h1, etc.) to anchor our context extraction
+ const block =
+ (node?.closest && node.closest(blockSelector)) || node || doc.body;
+
+ // Build ranges
+ const blockRange = doc.createRange();
+ blockRange.selectNodeContents(block);
+
+ const preRange = blockRange.cloneRange();
+ preRange.setEnd(range.startContainer, range.startOffset);
+
+ const postRange = blockRange.cloneRange();
+ postRange.setStart(range.endContainer, range.endOffset);
+
+ const preRaw = preRange.toString();
+ const matchRaw = range.toString();
+ const postRaw = postRange.toString();
+
+ // Consistent normalization across all three strings
+ const normalizeWhitespace = text => text.replace(/\s+/g, " ");
+ const preNorm = normalizeWhitespace(preRaw);
+ const matchNorm = normalizeWhitespace(matchRaw);
+ const postNorm = normalizeWhitespace(postRaw);
+
+ const start = preNorm.length; // offset in normalized block text
+ const end = start + matchNorm.length;
+ const sliceText = (text, start, end) => text.slice(start, end);
+
+ const beforeStr = sliceText(
+ preNorm,
+ Math.max(0, preNorm.length - contextRange),
+ preNorm.length
+ );
+ const afterStr = sliceText(postNorm, 0, contextRange);
+
+ return {
+ before: beforeStr,
+ match: matchNorm,
+ after: afterStr,
+ start,
+ end,
+ };
+ } catch {
+ return defaultResult;
+ }
+ }
+
+ /**
* Internal; check if an iteration request is available in the previous result
* that we cached.
*
@@ -534,9 +615,10 @@ export class FinderIterator {
* @param {Number} spawnId Since `stop()` is synchronous and this method
* is not, this identifier is used to learn if
* it's supposed to still continue after a pause.
+ * @param {Number} contextRange - Number of characters to extract before and after target string
* @yield {Range}
*/
- async _findAllRanges(finder, spawnId) {
+ async _findAllRanges(finder, spawnId, contextRange = 0) {
if (this._timeout) {
if (this._timer) {
clearTimeout(this._timer);
@@ -597,7 +679,13 @@ export class FinderIterator {
continue;
}
- listener.onIteratorRangeFound(range);
+ if (contextRange) {
+ listener.onIteratorRangeFound(range, {
+ context: this._extractSnippet(range, contextRange),
+ });
+ } else {
+ listener.onIteratorRangeFound(range);
+ }
if (limit !== -1 && --limit === 0) {
// We've reached our limit; no need to do more work for this listener.
diff --git a/toolkit/modules/FinderParent.sys.mjs b/toolkit/modules/FinderParent.sys.mjs
@@ -445,9 +445,13 @@ FinderParent.prototype = {
});
},
- requestMatchesCount(aSearchString, aLinksOnly) {
+ requestMatchesCount(aSearchString, optionalArgs) {
let list = this.gatherBrowsingContexts(this.browsingContext);
- let args = { searchString: aSearchString, linksOnly: aLinksOnly };
+ let args = {
+ searchString: aSearchString,
+ linksOnly: optionalArgs.linksOnly,
+ contextRange: optionalArgs.contextRange || 0,
+ };
args.useSubFrames = this.needSubFrameSearch(list);
@@ -509,6 +513,7 @@ FinderParent.prototype = {
let current = 0;
let total = 0;
let limit = 0;
+ let allSnippets = [];
for (let response of responses) {
// A null response can happen if another search was started
// and this one became invalid.
@@ -527,11 +532,22 @@ FinderParent.prototype = {
}
total += response.total;
limit = response.limit;
+
+ // Collect snippets from each response
+ if (response.snippets && Array.isArray(response.snippets)) {
+ allSnippets.push(...response.snippets);
+ }
}
if (sendNotification) {
this.callListeners("onMatchesCountResult", [
- { searchString: options.args.searchString, current, total, limit },
+ {
+ searchString: options.args.searchString,
+ current,
+ total,
+ limit,
+ snippets: allSnippets,
+ },
]);
}
}
diff --git a/toolkit/modules/tests/xpcshell/test_FinderIterator.js b/toolkit/modules/tests/xpcshell/test_FinderIterator.js
@@ -15,10 +15,54 @@ finderIterator._iterateDocument = function* () {
finderIterator._rangeStartsInLink = fakeRange => fakeRange.startsInLink;
-function FakeRange(textContent, startsInLink = false) {
- this.startContainer = {};
+function FakeRange(textContent, startsInLink = false, mockDocument = null) {
+ // Create a simple mock document if none provided
+ if (!mockDocument) {
+ mockDocument = {
+ body: {
+ textContent: `Some text before ${textContent} some text after`,
+ nodeName: "BODY",
+ },
+ createRange() {
+ // Return a minimal range object instead of calling FakeRange constructor
+ return {
+ selectNodeContents() {},
+ cloneRange() {
+ return this;
+ },
+ setEnd() {},
+ setStart() {},
+ toString() {
+ return "";
+ },
+ };
+ },
+ };
+ }
+
+ this.startContainer = {
+ nodeType: Node.TEXT_NODE,
+ ownerDocument: mockDocument,
+ parentElement: {
+ closest() {
+ return mockDocument.body;
+ },
+ nodeName: "P",
+ },
+ };
+ this.endContainer = this.startContainer;
+ this.commonAncestorContainer = this.startContainer;
+ this.startOffset = 10;
+ this.endOffset = 10 + textContent.length;
+ this.collapsed = false;
this.startsInLink = startsInLink;
this.toString = () => textContent;
+
+ this.cloneRange = () =>
+ new FakeRange(textContent, startsInLink, mockDocument);
+ this.selectNodeContents = () => {};
+ this.setEnd = () => {};
+ this.setStart = () => {};
}
var gMockWindow = {
@@ -53,7 +97,7 @@ add_task(async function test_start() {
entireWord: false,
finder: gMockFinder,
listener: {
- onIteratorRangeFound(range) {
+ onIteratorRangeFound(range, _extra) {
++count;
Assert.equal(range.toString(), findText, "Text content should match");
},
@@ -84,7 +128,7 @@ add_task(async function test_subframes() {
entireWord: false,
finder: gMockFinder,
listener: {
- onIteratorRangeFound(range) {
+ onIteratorRangeFound(range, _extra) {
++count;
Assert.equal(range.toString(), findText, "Text content should match");
},
@@ -307,7 +351,7 @@ add_task(async function test_reset() {
add_task(async function test_parallel_starts() {
let findText = "tak";
- let rangeCount = 2143;
+ let rangeCount = 4000;
prepareIterator(findText, rangeCount);
// Start off the iterator.
@@ -442,3 +486,74 @@ add_task(async function test_allowDistance() {
finderIterator.reset();
});
+
+add_task(async function test_context_extraction() {
+ let findText = "test";
+ let rangeCount = 5;
+ prepareIterator(findText, rangeCount);
+
+ let contexts = [];
+ await finderIterator.start({
+ caseSensitive: false,
+ entireWord: false,
+ finder: gMockFinder,
+ listener: {
+ onIteratorRangeFound(range, extra) {
+ if (extra && extra.context) {
+ contexts.push(extra.context);
+ }
+ },
+ },
+ matchDiacritics: false,
+ word: findText,
+ contextRange: 40,
+ });
+
+ Assert.equal(
+ contexts.length,
+ rangeCount,
+ "Should have context for each match"
+ );
+
+ // Test the structure of returned context
+ for (let context of contexts) {
+ Assert.strictEqual(typeof context, "object", "Context should be an object");
+ Assert.ok("before" in context, "Context should have 'before' property");
+ Assert.ok("match" in context, "Context should have 'match' property");
+ Assert.ok("after" in context, "Context should have 'after' property");
+ Assert.ok("start" in context, "Context should have 'start' property");
+ Assert.ok("end" in context, "Context should have 'end' property");
+ }
+
+ finderIterator.reset();
+});
+
+add_task(async function test_no_context_without_range() {
+ let findText = "test";
+ let rangeCount = 3;
+ prepareIterator(findText, rangeCount);
+
+ let contextsProvided = 0;
+ await finderIterator.start({
+ caseSensitive: false,
+ entireWord: false,
+ finder: gMockFinder,
+ listener: {
+ onIteratorRangeFound(range, extra) {
+ if (extra && extra.context) {
+ contextsProvided++;
+ }
+ },
+ },
+ matchDiacritics: false,
+ word: findText,
+ // Note: no contextRange parameter
+ });
+
+ Assert.equal(
+ contextsProvided,
+ 0,
+ "Should not provide context without contextRange"
+ );
+ finderIterator.reset();
+});
