commit 5486f5260b4e046a1b1d80e656327f48a14855ab
parent 7bbe749383b00366d48a5a7b34f24f6ae2e29800
Author: iulian moraru <imoraru@mozilla.com>
Date: Wed, 1 Oct 2025 22:25:42 +0300
Revert "Bug 1987972 - Expand context of target word in find flow appox 40 characters before and after r=Mardak,firefox-ai-ml-reviewers" for causing bc failures on browser_page_assist_actors.js.
This reverts commit e2918df3f45976be7a0a2dbc4cfb480c9cbb311c.
Diffstat:
8 files changed, 50 insertions(+), 428 deletions(-)
diff --git a/browser/components/genai/content/page-assist.css b/browser/components/genai/content/page-assist.css
@@ -6,10 +6,11 @@
padding: var(--space-large);
}
-.ai-response {
- white-space: pre-wrap;
+.prompt-textarea {
+ columns: 3;
+ width: 100%;
}
-#input.find-input {
- --input-background-icon: url(chrome://global/skin/icons/search-glass.svg);
+.ai-response {
+ white-space: pre-wrap;
}
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, ifDefined } from "chrome://global/content/vendor/lit.all.mjs";
+import { html } 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,37 +14,6 @@ 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;
@@ -55,10 +24,6 @@ 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() {
@@ -66,10 +31,6 @@ 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() {
@@ -84,16 +45,10 @@ 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;
@@ -102,35 +57,6 @@ 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;
@@ -188,7 +114,6 @@ export class PageAssist extends MozLitElement {
}
this._onTabSelect = () => {
- this._setupFinder();
const browser = gBrowser.selectedBrowser;
this.isCurrentPageReaderable = !!browser?.isArticle;
};
@@ -236,66 +161,11 @@ 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) {
@@ -326,29 +196,18 @@ export class PageAssist extends MozLitElement {
? html`<div class="ai-response">${this.aiResponse}</div>`
: ""}
<div>
- <page-assists-input
- class="find-input"
- type="text"
- placeholder="Find in page..."
- .value=${this.userPrompt}
- @input=${this._handlePromptInput}
- ></page-assists-input>
- </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>`
- : ""}
+ <textarea
+ class="prompt-textarea"
+ @input=${e => this._handlePromptInput(e)}
+ ></textarea>
+ <moz-button
+ id="submit-user-prompt-btn"
+ type="primary"
+ size="small"
+ @click=${this._handleSubmit}
+ >
+ Submit
+ </moz-button>
</div>
</div>
</div>
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, {
- linksOnly: data.linksOnly,
- useSubFrames: data.useSubFrames,
- contextRange: data.contextRange,
- })
+ .requestMatchesCount(
+ data.searchString,
+ data.linksOnly,
+ data.useSubFrames
+ )
.then(result => {
if (result) {
result.browsingContextId = this.browsingContext.id;
diff --git a/toolkit/content/widgets/findbar.js b/toolkit/content/widgets/findbar.js
@@ -500,9 +500,10 @@
return;
}
- this.browser.finder.requestMatchesCount(this._findField.value, {
- linksOnly: this.findMode == this.FIND_LINKS,
- });
+ this.browser.finder.requestMatchesCount(
+ this._findField.value,
+ 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, {
- linksOnly: aArgs.linksOnly,
- useSubFrames: aArgs.useSubFrames,
- });
+ let matchCountPromise = this.requestMatchesCount(
+ aArgs.searchString,
+ aArgs.linksOnly,
+ aArgs.useSubFrames
+ );
let results = await Promise.all([highlightPromise, matchCountPromise]);
@@ -606,7 +606,7 @@ Finder.prototype = {
return result;
},
- async requestMatchesCount(aWord, optionalArgs) {
+ async requestMatchesCount(aWord, aLinksOnly, aUseSubFrames = true) {
if (
this._lastFindResult == Ci.nsITypeAheadFind.FIND_NOTFOUND ||
this.searchString == "" ||
@@ -624,15 +624,11 @@ Finder.prototype = {
let params = {
caseSensitive: this._fastFind.caseSensitive,
entireWord: this._fastFind.entireWord,
- linksOnly: optionalArgs.linksOnly || false,
+ linksOnly: aLinksOnly,
matchDiacritics: this._fastFind.matchDiacritics,
word: aWord,
- useSubFrames:
- optionalArgs.useSubFrames !== undefined
- ? optionalArgs.useSubFrames
- : true,
+ useSubFrames: aUseSubFrames,
};
-
if (!this.iterator.continueRunning(params)) {
this.iterator.stop();
}
@@ -643,11 +639,7 @@ Finder.prototype = {
limit: this.matchesCountLimit,
listener: this,
useCache: true,
- useSubFrames:
- optionalArgs.useSubFrames !== undefined
- ? optionalArgs.useSubFrames
- : true,
- contextRange: optionalArgs.contextRange || 0,
+ useSubFrames: aUseSubFrames,
})
);
@@ -662,22 +654,13 @@ Finder.prototype = {
// FinderIterator listener implementation
- onIteratorRangeFound(range, extra) {
+ onIteratorRangeFound(range) {
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 =
@@ -692,10 +675,7 @@ 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,8 +78,7 @@ export class FinderIterator {
* Optional, defaults to `false`.
* @param {Object} options.listener Listener object that implements the
* following callback functions:
- * - onIteratorRangeFound({Range} range, {Object} extra);
- * extra.context contains text snippet around the match
+ * - onIteratorRangeFound({Range} range);
* - onIteratorReset();
* - onIteratorRestart({Object} iterParams);
* - onIteratorStart({Object} iterParams);
@@ -91,7 +90,6 @@ 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({
@@ -106,7 +104,6 @@ export class FinderIterator {
useCache,
word,
useSubFrames,
- contextRange,
}) {
// Take care of default values for non-required options.
if (typeof allowDistance != "number") {
@@ -202,7 +199,7 @@ export class FinderIterator {
// Start!
this.running = true;
this._currentParams = iterParams;
- this._findAllRanges(finder, ++this._spawnId, contextRange);
+ this._findAllRanges(finder, ++this._spawnId);
return promise;
}
@@ -373,84 +370,6 @@ 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.
*
@@ -615,10 +534,9 @@ 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, contextRange = 0) {
+ async _findAllRanges(finder, spawnId) {
if (this._timeout) {
if (this._timer) {
clearTimeout(this._timer);
@@ -679,13 +597,7 @@ export class FinderIterator {
continue;
}
- if (contextRange) {
- listener.onIteratorRangeFound(range, {
- context: this._extractSnippet(range, contextRange),
- });
- } else {
- listener.onIteratorRangeFound(range);
- }
+ 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,13 +445,9 @@ FinderParent.prototype = {
});
},
- requestMatchesCount(aSearchString, optionalArgs) {
+ requestMatchesCount(aSearchString, aLinksOnly) {
let list = this.gatherBrowsingContexts(this.browsingContext);
- let args = {
- searchString: aSearchString,
- linksOnly: optionalArgs.linksOnly,
- contextRange: optionalArgs.contextRange || 0,
- };
+ let args = { searchString: aSearchString, linksOnly: aLinksOnly };
args.useSubFrames = this.needSubFrameSearch(list);
@@ -513,7 +509,6 @@ 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.
@@ -532,22 +527,11 @@ 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,
- snippets: allSnippets,
- },
+ { searchString: options.args.searchString, current, total, limit },
]);
}
}
diff --git a/toolkit/modules/tests/xpcshell/test_FinderIterator.js b/toolkit/modules/tests/xpcshell/test_FinderIterator.js
@@ -15,54 +15,10 @@ finderIterator._iterateDocument = function* () {
finderIterator._rangeStartsInLink = fakeRange => fakeRange.startsInLink;
-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;
+function FakeRange(textContent, startsInLink = false) {
+ this.startContainer = {};
this.startsInLink = startsInLink;
this.toString = () => textContent;
-
- this.cloneRange = () =>
- new FakeRange(textContent, startsInLink, mockDocument);
- this.selectNodeContents = () => {};
- this.setEnd = () => {};
- this.setStart = () => {};
}
var gMockWindow = {
@@ -97,7 +53,7 @@ add_task(async function test_start() {
entireWord: false,
finder: gMockFinder,
listener: {
- onIteratorRangeFound(range, _extra) {
+ onIteratorRangeFound(range) {
++count;
Assert.equal(range.toString(), findText, "Text content should match");
},
@@ -128,7 +84,7 @@ add_task(async function test_subframes() {
entireWord: false,
finder: gMockFinder,
listener: {
- onIteratorRangeFound(range, _extra) {
+ onIteratorRangeFound(range) {
++count;
Assert.equal(range.toString(), findText, "Text content should match");
},
@@ -351,7 +307,7 @@ add_task(async function test_reset() {
add_task(async function test_parallel_starts() {
let findText = "tak";
- let rangeCount = 4000;
+ let rangeCount = 2143;
prepareIterator(findText, rangeCount);
// Start off the iterator.
@@ -486,74 +442,3 @@ 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();
-});
