From 4baf0b41fb4eca3c63cd5eede4f23eda2b565185 Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Mon, 24 Aug 2026 20:02:38 +0200 Subject: [PATCH 1/2] Ask about old mirrors only when there are some The import offer was gated on a "have I asked" preference and nothing else, so it greeted every fresh install with a question about mirrors that do not exist, stacked on the storage-permission dialog it was shown beside. A user who answered yes before settling that permission copied into app-private storage, which an uninstall erases. StoragePaths.legacyMirrors() looks for the Download/HTTrack/Websites folder that builds before versionCode 61 wrote to, and returns it only when it holds a project. Without access the shared root is null, so it finds nothing and nothing is asked: a question the user has no way to answer is worse than never offering the import. onResume() asks again after a grant, which is the first moment the folder can be seen, and by then the destination resolves to shared storage rather than the private one. The offer is the only thing gated. The mirrors themselves were never unreachable: builds 61 to 63, which is what production has served since 2017, already wrote to the shared HTTrack/ root this build reads. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Xavier Roche --- .../com/httrack/android/HTTrackActivity.java | 12 +++++- .../com/httrack/android/StoragePaths.java | 26 ++++++++++++ .../android/LegacyMirrorImportTest.java | 14 +++++++ .../com/httrack/android/StoragePathsTest.java | 40 +++++++++++++++++++ 4 files changed, 91 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/httrack/android/HTTrackActivity.java b/app/src/main/java/com/httrack/android/HTTrackActivity.java index a5c8833d..b357304e 100755 --- a/app/src/main/java/com/httrack/android/HTTrackActivity.java +++ b/app/src/main/java/com/httrack/android/HTTrackActivity.java @@ -800,7 +800,8 @@ protected void onCreate(final Bundle savedInstanceState) { } // First launch only, so a rotation does not bring the offer back; and not stacked on top - // of the native-load failure dialog. + // of the native-load failure dialog. Silent unless mirrors are already visible; onResume() + // asks again once a storage grant makes them so. if (savedInstanceState == null && HTTrackLib.loadedSuccessfully()) { offerLegacyMirrorImportOnce(); } @@ -2790,6 +2791,11 @@ private void offerLegacyMirrorImportOnce() { if (settings.getBoolean(IMPORT_OFFERED_NAME, false)) { return; } + // Nothing found means nothing asked: without access we cannot look, and a question the user + // has no way to answer is worse than never offering the import at all. + if (StoragePaths.legacyMirrors(sharedStorageRoot()) == null) { + return; + } new AlertDialog.Builder(this) .setMessage(R.string.import_mirrors_offer) .setPositiveButton(R.string.import_mirrors_offer_yes, (dialog, which) -> { @@ -3352,6 +3358,10 @@ protected void onResume() { } // A grant made on the settings screen flips the button/warning off (no-op off this panel). refreshStorageAccessHints(); + // A grant is the first moment we can see the legacy folder, so this is where the offer belongs. + if (runner == null) { + offerLegacyMirrorImportOnce(); + } } @Override diff --git a/app/src/main/java/com/httrack/android/StoragePaths.java b/app/src/main/java/com/httrack/android/StoragePaths.java index 4865a22e..acaacaba 100644 --- a/app/src/main/java/com/httrack/android/StoragePaths.java +++ b/app/src/main/java/com/httrack/android/StoragePaths.java @@ -77,6 +77,32 @@ static File defaultRoot(final File external, final File internal, final File sha return new File(external != null ? external : internal, "Websites"); } + /** + * Mirrors left by builds before versionCode 61, which wrote under Download/ rather than the + * shared HTTrack/ root every build since uses. Returns the folder only when it holds at least + * one project, so a caller can stay silent rather than ask about nothing. + * + * @param shared + * the shared storage root, null when we have no access to look + * @return the legacy folder, or null when there is nothing to offer + */ + static File legacyMirrors(final File shared) { + if (shared == null) { + return null; + } + final File legacy = new File(new File(new File(shared, "Download"), "HTTrack"), "Websites"); + final File[] entries = legacy.listFiles(); + if (entries == null) { + return null; + } + for (final File entry : entries) { + if (entry.isDirectory()) { + return legacy; + } + } + return null; + } + /** * Whether the freshly resolved root differs from the one in use, the first resolution counting * as a move. Gates the work that must happen once per move, not once per resume. diff --git a/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java b/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java index 1ccbe5d1..45cf6701 100644 --- a/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java +++ b/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java @@ -370,4 +370,18 @@ public void aByteForByteCopyMatchesBinaryContent() throws Exception { assertArrayEquals(raw, Files.readAllBytes(new File(dest, "blob.bin").toPath())); } + + /** + * Without access the shared root is null, so the detector cannot look and the offer must not + * appear. Only the wiring can say that; the detector alone cannot tell "no access" from "empty". + */ + @Test + public void theOfferIsGatedOnFindingSomething() throws IOException { + final String body = TestSources.between(TestSources.javaSource("HTTrackActivity"), + "private void offerLegacyMirrorImportOnce", "new AlertDialog.Builder"); + assertTrue("the offer must ask the detector before it shows anything", + body.contains("StoragePaths.legacyMirrors(sharedStorageRoot())")); + assertTrue("finding nothing must return before the dialog is built", + body.contains("== null") && body.contains("return;")); + } } diff --git a/app/src/test/java/com/httrack/android/StoragePathsTest.java b/app/src/test/java/com/httrack/android/StoragePathsTest.java index 7c273983..90e8d16e 100644 --- a/app/src/test/java/com/httrack/android/StoragePathsTest.java +++ b/app/src/test/java/com/httrack/android/StoragePathsTest.java @@ -5,6 +5,7 @@ import static org.junit.Assert.assertTrue; import java.io.File; +import java.io.IOException; import java.nio.file.Files; import org.junit.Before; @@ -129,4 +130,43 @@ public void refusesAPathOutsideTheSharedRootOrWithoutIt() throws Exception { assertNull(StoragePaths.externalStorageDocId(shared, shared)); // the root itself, no sub-path assertNull(StoragePaths.externalStorageDocId(new File(shared, "HTTrack"), null)); } + + private static File legacyTree(final File shared, final boolean withProject) throws IOException { + final File websites = new File(new File(new File(shared, "Download"), "HTTrack"), "Websites"); + assertTrue(websites.mkdirs()); + if (withProject) { + assertTrue(new File(websites, "someproject").mkdir()); + } + return websites; + } + + /** A question the user cannot answer is worse than no question, so silence is the default. */ + @Test + public void anAbsentLegacyFolderIsNotOffered() throws IOException { + assertNull(StoragePaths.legacyMirrors(tmp.newFolder("bare"))); + } + + /** Builds before versionCode 61 wrote here; an empty folder is not a reason to ask. */ + @Test + public void anEmptyLegacyFolderIsNotOffered() throws IOException { + final File shared = tmp.newFolder("empty"); + legacyTree(shared, false); + assertNull(StoragePaths.legacyMirrors(shared)); + } + + /** A stray file is not a project; only a project directory earns the question. */ + @Test + public void aFileIsNotAProject() throws IOException { + final File shared = tmp.newFolder("stray"); + final File websites = legacyTree(shared, false); + assertTrue(new File(websites, "notes.txt").createNewFile()); + assertNull(StoragePaths.legacyMirrors(shared)); + } + + @Test + public void aLegacyProjectIsOffered() throws IOException { + final File shared = tmp.newFolder("real"); + final File websites = legacyTree(shared, true); + assertEquals(websites, StoragePaths.legacyMirrors(shared)); + } } From b1947cc1396af494d7c02d2a233626518641b084 Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Mon, 24 Aug 2026 20:16:35 +0200 Subject: [PATCH 2/2] Ask once, on the resume that can answer Review found three ways the offer misbehaved. On a cold launch with mirrors present, onCreate showed it and onResume immediately showed a second one on top; tapping Yes on either opened its own document picker. "Not now" leaves the latch clear by design, so the question came back on every return to the app, including from Options or Help. And rotation skipped the onCreate call but not the onResume one, defeating the guard whose comment says a rotation must not bring the offer back. The latch records that the user decided, not that we asked. So the resume call now fires only when access has just appeared, which is the one resume that can reveal anything; onCreate covers access granted before launch. An empty project directory no longer counts as something to import, since asking about it leads to a copy of nothing. The Javadoc on the offer said no permission remains to detect the folder, which this change makes false. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Xavier Roche --- .../com/httrack/android/HTTrackActivity.java | 16 ++++-- .../com/httrack/android/StoragePaths.java | 56 ++++++++++++------- .../android/LegacyMirrorImportTest.java | 21 +++++-- .../com/httrack/android/StoragePathsTest.java | 49 ++++++++++++---- 4 files changed, 99 insertions(+), 43 deletions(-) diff --git a/app/src/main/java/com/httrack/android/HTTrackActivity.java b/app/src/main/java/com/httrack/android/HTTrackActivity.java index b357304e..9c341a26 100755 --- a/app/src/main/java/com/httrack/android/HTTrackActivity.java +++ b/app/src/main/java/com/httrack/android/HTTrackActivity.java @@ -143,6 +143,9 @@ public class HTTrackActivity extends FragmentActivity { protected static final String NOTIFY_ASKED_NAME = "NotificationPermissionAsked"; // Whether the one-time "import your old mirrors" offer has been shown and dismissed for good. protected static final String IMPORT_OFFERED_NAME = "LegacyImportOffered"; + + /* Access as of the last resume, so a grant can be told from a plain return to the app. */ + private boolean hadStorageAccess; // Whether all-files storage access was ever offered; a refusal keeps mirrors in private storage. protected static final String STORAGE_ASKED_NAME = "StorageAccessAsked"; // The root we left on the last move: nothing is migrated, so the older projects are still there. @@ -802,6 +805,7 @@ protected void onCreate(final Bundle savedInstanceState) { // First launch only, so a rotation does not bring the offer back; and not stacked on top // of the native-load failure dialog. Silent unless mirrors are already visible; onResume() // asks again once a storage grant makes them so. + hadStorageAccess = hasAllFilesAccess(); if (savedInstanceState == null && HTTrackLib.loadedSuccessfully()) { offerLegacyMirrorImportOnce(); } @@ -2791,9 +2795,8 @@ private void offerLegacyMirrorImportOnce() { if (settings.getBoolean(IMPORT_OFFERED_NAME, false)) { return; } - // Nothing found means nothing asked: without access we cannot look, and a question the user - // has no way to answer is worse than never offering the import at all. - if (StoragePaths.legacyMirrors(sharedStorageRoot()) == null) { + // Null both without access and with nothing to import; either way, stay silent. + if (StoragePaths.legacyMirrorRoot(sharedStorageRoot()) == null) { return; } new AlertDialog.Builder(this) @@ -3358,10 +3361,13 @@ protected void onResume() { } // A grant made on the settings screen flips the button/warning off (no-op off this panel). refreshStorageAccessHints(); - // A grant is the first moment we can see the legacy folder, so this is where the offer belongs. - if (runner == null) { + // Ask again once a grant may have surfaced the folder; not mid-crawl, and not on a plain + // resume, which onCreate has already covered. + final boolean access = hasAllFilesAccess(); + if (runner == null && StoragePaths.accessJustAppeared(hadStorageAccess, access)) { offerLegacyMirrorImportOnce(); } + hadStorageAccess = access; } @Override diff --git a/app/src/main/java/com/httrack/android/StoragePaths.java b/app/src/main/java/com/httrack/android/StoragePaths.java index acaacaba..b3ce7cb6 100644 --- a/app/src/main/java/com/httrack/android/StoragePaths.java +++ b/app/src/main/java/com/httrack/android/StoragePaths.java @@ -78,29 +78,16 @@ static File defaultRoot(final File external, final File internal, final File sha } /** - * Mirrors left by builds before versionCode 61, which wrote under Download/ rather than the - * shared HTTrack/ root every build since uses. Returns the folder only when it holds at least - * one project, so a caller can stay silent rather than ask about nothing. + * Whether a resume is the moment storage access appeared, which is the only resume that can + * newly reveal anything. Every other one must stay silent rather than re-ask. * - * @param shared - * the shared storage root, null when we have no access to look - * @return the legacy folder, or null when there is nothing to offer + * @param hadAccess + * access as it stood at the previous check + * @param hasAccess + * access as it stands now */ - static File legacyMirrors(final File shared) { - if (shared == null) { - return null; - } - final File legacy = new File(new File(new File(shared, "Download"), "HTTrack"), "Websites"); - final File[] entries = legacy.listFiles(); - if (entries == null) { - return null; - } - for (final File entry : entries) { - if (entry.isDirectory()) { - return legacy; - } - } - return null; + static boolean accessJustAppeared(final boolean hadAccess, final boolean hasAccess) { + return hasAccess && !hadAccess; } /** @@ -181,6 +168,33 @@ static File resolveRoot(final File base, final Boolean writable, final File defa return defaultRoot; } + /** + * Mirrors left by builds before versionCode 61, which wrote under Download/ rather than the + * shared HTTrack/ root every build since uses. Returns the folder only when it holds a project + * with something in it, so a caller can stay silent rather than ask about nothing. + * + * @param shared + * the shared storage root, null when we have no access to look + * @return the legacy folder, or null when there is nothing to offer + */ + static File legacyMirrorRoot(final File shared) { + if (shared == null) { + return null; + } + final File legacy = new File(new File(new File(shared, "Download"), "HTTrack"), "Websites"); + final File[] entries = legacy.listFiles(); + if (entries == null) { + return null; + } + for (final File entry : entries) { + final String[] project = entry.list(); + if (project != null && project.length != 0) { + return legacy; + } + } + return null; + } + /** * Documents-provider id ("primary:<relative>") to open {@code dir} in the system Files app via * ACTION_VIEW, or null when it lies outside the primary shared volume or inside the Android/ subtree diff --git a/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java b/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java index 45cf6701..2b4ed91d 100644 --- a/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java +++ b/app/src/test/java/com/httrack/android/LegacyMirrorImportTest.java @@ -376,12 +376,23 @@ public void aByteForByteCopyMatchesBinaryContent() throws Exception { * appear. Only the wiring can say that; the detector alone cannot tell "no access" from "empty". */ @Test - public void theOfferIsGatedOnFindingSomething() throws IOException { + public void findingNothingReturnsBeforeTheDialog() throws IOException { final String body = TestSources.between(TestSources.javaSource("HTTrackActivity"), "private void offerLegacyMirrorImportOnce", "new AlertDialog.Builder"); - assertTrue("the offer must ask the detector before it shows anything", - body.contains("StoragePaths.legacyMirrors(sharedStorageRoot())")); - assertTrue("finding nothing must return before the dialog is built", - body.contains("== null") && body.contains("return;")); + // One pattern, so the test cannot be satisfied by a stray "== null" and the pref check's + // own "return;" the way two independent substrings could. + assertTrue("finding nothing must return before the dialog is built", body.matches( + "(?s).*StoragePaths\\.legacyMirrorRoot\\(sharedStorageRoot\\(\\)\\)\\s*==\\s*null\\)\\s*\\{\\s*return;.*")); + } + + /** The resume that brings access is the only one that can reveal the folder. */ + @Test + public void theResumeOfferIsGatedOnTheGrant() throws IOException { + final String body = TestSources.between(TestSources.javaSource("HTTrackActivity"), + "protected void onResume", "\n }"); + assertTrue("a plain resume must not re-ask", body.matches( + "(?s).*StoragePaths\\.accessJustAppeared\\([^)]*\\)\\)\\s*\\{\\s*offerLegacyMirrorImportOnce\\(\\);.*")); + assertTrue("the next resume must compare against this one", + body.contains("hadStorageAccess = access;")); } } diff --git a/app/src/test/java/com/httrack/android/StoragePathsTest.java b/app/src/test/java/com/httrack/android/StoragePathsTest.java index 90e8d16e..5be20fed 100644 --- a/app/src/test/java/com/httrack/android/StoragePathsTest.java +++ b/app/src/test/java/com/httrack/android/StoragePathsTest.java @@ -1,6 +1,7 @@ package com.httrack.android; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -131,42 +132,66 @@ public void refusesAPathOutsideTheSharedRootOrWithoutIt() throws Exception { assertNull(StoragePaths.externalStorageDocId(new File(shared, "HTTrack"), null)); } - private static File legacyTree(final File shared, final boolean withProject) throws IOException { + /** The path builds before versionCode 61 wrote to, spelled out so a change to it fails here. */ + private static File legacyTree(final File shared) throws IOException { final File websites = new File(new File(new File(shared, "Download"), "HTTrack"), "Websites"); assertTrue(websites.mkdirs()); - if (withProject) { - assertTrue(new File(websites, "someproject").mkdir()); - } return websites; } + private static void project(final File websites, final String name) throws IOException { + final File dir = new File(websites, name); + assertTrue(dir.mkdir()); + assertTrue(new File(dir, "index.html").createNewFile()); + } + /** A question the user cannot answer is worse than no question, so silence is the default. */ @Test public void anAbsentLegacyFolderIsNotOffered() throws IOException { - assertNull(StoragePaths.legacyMirrors(tmp.newFolder("bare"))); + assertNull(StoragePaths.legacyMirrorRoot(tmp.newFolder("bare"))); } /** Builds before versionCode 61 wrote here; an empty folder is not a reason to ask. */ @Test public void anEmptyLegacyFolderIsNotOffered() throws IOException { final File shared = tmp.newFolder("empty"); - legacyTree(shared, false); - assertNull(StoragePaths.legacyMirrors(shared)); + legacyTree(shared); + assertNull(StoragePaths.legacyMirrorRoot(shared)); + } + + /** An empty project would import nothing, so it is not worth a question either. */ + @Test + public void anEmptyProjectIsNotOffered() throws IOException { + final File shared = tmp.newFolder("hollow"); + assertTrue(new File(legacyTree(shared), "someproject").mkdir()); + assertNull(StoragePaths.legacyMirrorRoot(shared)); } /** A stray file is not a project; only a project directory earns the question. */ @Test public void aFileIsNotAProject() throws IOException { final File shared = tmp.newFolder("stray"); - final File websites = legacyTree(shared, false); - assertTrue(new File(websites, "notes.txt").createNewFile()); - assertNull(StoragePaths.legacyMirrors(shared)); + assertTrue(new File(legacyTree(shared), "notes.txt").createNewFile()); + assertNull(StoragePaths.legacyMirrorRoot(shared)); } + /** The one test that pins the path itself: every negative case passes on a wrong path too. */ @Test public void aLegacyProjectIsOffered() throws IOException { final File shared = tmp.newFolder("real"); - final File websites = legacyTree(shared, true); - assertEquals(websites, StoragePaths.legacyMirrors(shared)); + final File websites = legacyTree(shared); + project(websites, "someproject"); + assertEquals(websites, StoragePaths.legacyMirrorRoot(shared)); + } + + /** Only the resume that brings access can reveal anything; every other must stay silent. */ + @Test + public void onlyTheGrantItselfRaisesTheOffer() { + assertTrue(StoragePaths.accessJustAppeared(false, true)); + assertFalse("a plain resume with access must not re-ask", + StoragePaths.accessJustAppeared(true, true)); + assertFalse(StoragePaths.accessJustAppeared(false, false)); + assertFalse("access going away is not an invitation", + StoragePaths.accessJustAppeared(true, false)); } }