commit c2fe159de9b4889254fd9c523f6534923810746a
parent efaab113e1b76f6e75897402d9802fa5ae684ca1
Author: Jason Prickett <jprickett@mozilla.com>
Date: Thu, 9 Oct 2025 01:19:33 +0000
Bug 1992157 - Set backup file to restore to the newest file when multiple files are found r=omc-reviewers,sthompson,mviar
Differential Revision: https://phabricator.services.mozilla.com/D267292
Diffstat:
6 files changed, 79 insertions(+), 11 deletions(-)
diff --git a/browser/components/aboutwelcome/actors/AboutWelcomeChild.sys.mjs b/browser/components/aboutwelcome/actors/AboutWelcomeChild.sys.mjs
@@ -201,9 +201,11 @@ export class AboutWelcomeChild extends JSWindowActorChild {
);
}
- AWFindBackupsInWellKnownLocations() {
- // This return value will be used in https://bugzilla.mozilla.org/show_bug.cgi?id=1992157
- return this.sendQueryAndCloneForContent("AWPage:BACKUP_FIND_WELL_KNOWN");
+ AWFindBackupsInWellKnownLocations(data) {
+ return this.sendQueryAndCloneForContent(
+ "AWPage:BACKUP_FIND_WELL_KNOWN",
+ data
+ );
}
/**
diff --git a/browser/components/aboutwelcome/actors/AboutWelcomeParent.sys.mjs b/browser/components/aboutwelcome/actors/AboutWelcomeParent.sys.mjs
@@ -298,7 +298,7 @@ export class AboutWelcomeParent extends JSWindowActorParent {
} catch {
bs = lazy.BackupService.init();
}
- return bs.findBackupsInWellKnownLocations();
+ return bs.findBackupsInWellKnownLocations(data);
}
default:
lazy.log.debug(`Unexpected event ${type} was not handled.`);
diff --git a/browser/components/aboutwelcome/content-src/components/EmbeddedBackupRestore.jsx b/browser/components/aboutwelcome/content-src/components/EmbeddedBackupRestore.jsx
@@ -12,7 +12,10 @@ export const EmbeddedBackupRestore = ({ handleAction, skipButton }) => {
useEffect(() => {
const loadRestore = async () => {
- await window.AWFindBackupsInWellKnownLocations?.();
+ await window.AWFindBackupsInWellKnownLocations?.({
+ validateFile: true,
+ multipleFiles: true,
+ });
};
loadRestore();
// Clear the pref used to target the restore screen so that users will not
diff --git a/browser/components/aboutwelcome/content/aboutwelcome.bundle.js b/browser/components/aboutwelcome/content/aboutwelcome.bundle.js
@@ -3756,7 +3756,10 @@ const EmbeddedBackupRestore = ({
const ref = (0,react__WEBPACK_IMPORTED_MODULE_0__.useRef)(null);
(0,react__WEBPACK_IMPORTED_MODULE_0__.useEffect)(() => {
const loadRestore = async () => {
- await window.AWFindBackupsInWellKnownLocations?.();
+ await window.AWFindBackupsInWellKnownLocations?.({
+ validateFile: true,
+ multipleFiles: true
+ });
};
loadRestore();
// Clear the pref used to target the restore screen so that users will not
diff --git a/browser/components/backup/BackupService.sys.mjs b/browser/components/backup/BackupService.sys.mjs
@@ -3827,6 +3827,25 @@ export class BackupService extends EventTarget {
return { multipleBackupsFound: true };
}
+ // Sort the files by the timestamp at the end of the filename,
+ // so the newest valid file is selected as the file to restore
+ if (multipleFiles && maybeBackupFiles.length > 1 && validateFile) {
+ maybeBackupFiles.sort((a, b) => {
+ let nameA = PathUtils.filename(a);
+ let nameB = PathUtils.filename(b);
+ const match = /_(\d{8}-\d{4})\.html$/;
+ let timestampA = nameA.match(match)?.[1];
+ let timestampB = nameB.match(match)?.[1];
+
+ // If either file doesn't match the expected pattern, maintain the original order
+ if (!timestampA || !timestampB) {
+ return 0;
+ }
+
+ return timestampB.localeCompare(timestampA);
+ });
+ }
+
for (const file of maybeBackupFiles) {
if (validateFile) {
try {
@@ -3853,6 +3872,13 @@ export class BackupService extends EventTarget {
this.#_state.backupFileToRestore = file;
this.stateUpdate();
+ // In the case that multiple files were found,
+ // but we also validated files to set the newest backup file as the file to restore,
+ // we still want to return that multiple backups were found.
+ if (multipleFiles && maybeBackupFiles.length > 1 && validateFile) {
+ return { multipleBackupsFound: true };
+ }
+
// TODO: support multiple valid backups for different profiles.
// Currently, we break out of the loop and select the first profile that works.
// We want to eventually support showing multiple valid profiles to the user.
@@ -3879,17 +3905,27 @@ export class BackupService extends EventTarget {
* - Clears any existing `lastBackupFileName` and `backupFileToRestore`
* in the internal state prior to searching.
*
+ * @param {object} [options] - Configuration options.
+ * @param {boolean} [options.validateFile=false] - Whether to validate each backup file
+ * before selecting it.
+ * @param {boolean} [options.multipleFiles=false] - Whether to allow selecting a file
+ * when multiple files are found
+ *
* @returns {Promise<object>} A result object with the following properties:
* - {boolean} found — Whether a backup file was found.
* - {string|null} backupFileToRestore — Path or identifier of the backup file (if found).
* - {boolean} multipleBackupsFound — Currently always `false`, reserved for future use.
*/
- async findBackupsInWellKnownLocations() {
+ async findBackupsInWellKnownLocations({
+ validateFile = false,
+ multipleFiles = false,
+ } = {}) {
this.#_state.lastBackupFileName = "";
this.#_state.backupFileToRestore = null;
let { multipleBackupsFound } = await this.findIfABackupFileExists({
- validateFile: false,
+ validateFile,
+ multipleFiles,
});
// if a valid backup file was found, backupFileToRestore should be set
diff --git a/browser/components/backup/tests/xpcshell/test_backupService_findBackupsInWellKnownLocations.js b/browser/components/backup/tests/xpcshell/test_backupService_findBackupsInWellKnownLocations.js
@@ -29,6 +29,10 @@ add_task(
.stub(bs, "resolveExistingArchiveDestFolderPath")
.callsFake(async _configured => BACKUP_DIR);
+ // getBackupFileInfo should return without throwing to simulate
+ // what happens when a valid backup file's validity is checked
+ sandbox.stub(bs, "getBackupFileInfo").callsFake(async _filePath => {});
+
// Sanity: the directory exists and is empty
Assert.ok(await IOUtils.exists(BACKUP_DIR), "Backup directory exists");
Assert.equal(
@@ -38,7 +42,7 @@ add_task(
);
// 1) Single valid file -> findBackupsInWellKnownLocations should find it
- const ONE = "FirefoxBackup_one.html";
+ const ONE = "FirefoxBackup_one_20241201-1200.html";
await touch(ONE);
let result = await bs.findBackupsInWellKnownLocations();
@@ -55,13 +59,13 @@ add_task(
);
// 2) Add a second matching file -> well-known search should refuse to pick (validateFile=false)
- const TWO = "FirefoxBackup_two.html";
+ const TWO = "FirefoxBackup_two_20241202-1300.html";
await touch(TWO);
let result2 = await bs.findBackupsInWellKnownLocations();
Assert.ok(
!result2.found,
- "Found should be false when multiple candidates exist"
+ "Found should be false when multiple candidates exist and validateFile=false"
);
Assert.equal(
result2.multipleBackupsFound,
@@ -81,6 +85,26 @@ add_task(
});
Assert.ok(!multipleBackupsFound, "Should not report multiple when allowed");
+ // 4) With validateFile=true and multipleFiles=true, should select newest file,
+ // but still report multipleBackupsFound=true
+ let result3 = await bs.findBackupsInWellKnownLocations({
+ validateFile: true,
+ multipleFiles: true,
+ });
+ Assert.ok(
+ result3.found,
+ "Found should be true when validateFile=true and multiple files exist"
+ );
+ Assert.equal(
+ result3.multipleBackupsFound,
+ true,
+ "Should signal multipleBackupsFound when validateFile=true and multipleFiles=true and multiple files exist"
+ );
+ Assert.ok(
+ result3.backupFileToRestore && result3.backupFileToRestore.endsWith(TWO),
+ "Should select the newest file when validateFile=true"
+ );
+
// Cleanup
sandbox.restore();
await IOUtils.remove(TEST_ROOT, { recursive: true });
