Option edits are lost whenever the options screen is recreated - #131
Merged
Conversation
OptionsActivity rebuilt its state from the launching intent on every recreation, so a rotation or a low-memory kill while the user was off in Help or the file chooser reverted every unsaved edit and handed the stale map back on Back. Save the option map and the open tab into the instance bundle and read them back, using the same hooks HTTrackActivity has. The visible tab is flushed into the map first, because a tab's widgets only reach the map when that tab is left; restore goes through setPane(), which covers the tablet detail pane as well as the phone path. Closes #129 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
The bundle keys were literals repeated on the write and the read side, so a mismatch could only be caught by scanning the source; name them instead and let the compiler pair them. That test is dropped rather than strengthened. Fold the "no map, restore nothing" and stale-index rules into paneToRestore(), the one part of the round trip that can run under the stub android.jar, and test it against real values. Neither Bundle nor SparseArray is mocked there, so the map round trip itself stays source-read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
A saved options bundle carried no build stamp, so a bundle written before a Play update was restored into the new build, where R.id has been renumbered: the map came back keyed by the old build's ints and finish() handed it to HTTrackActivity, which can write it to the project profile. Stamp the bundle with versionCode and refuse it on mismatch, mirroring what HTTrackActivity already does. The pane bounds check goes with it -- a stale index can no longer arrive, and it could never have caught a reordered tab list anyway. The save/restore pair moves to OptionsInstanceState, behind two seams: a Store over the Bundle and a Screen over the activity. Both directions now run in a plain JUnit test, over a HashMap Store that answers the default on a type mismatch the way Bundle does, so the key constants are load-bearing there. Dropping the map write, swapping the map and pane keys, and dropping the version guard each fail it. The three tests that only matched source text are gone, except the one holding the two Activity overrides to calling in: no seam reaches those. Also names the map's intent-extra key, shared by both ends of the contract, and lifts the PackageInfo lookup out of HTTrackActivity.onCreate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
visiblePane() had swallowed the tabClasses loop behind the Screen interface, where only the test's fake stands, so gutting the method to return NO_PANE left the whole suite green. The loop moves back into a static paneIndexOf(Class) the test calls directly. That move had also dropped the bounds check on the restored index, and setPane() indexes tabClasses unguarded, so an index naming no tab threw inside onRestoreInstanceState. The version stamp keeps such a bundle off Play, but not off a rebuild at the same versionCode: an "adb install -r" of a debug build with a reordered tab list. Screen now reports its tab count, and restore() leaves the menu when the saved index falls outside it. Also scope the lifecycle-hook assertions back to the two override bodies, where they no longer pass on a call moved elsewhere in the file, and drop OptionsInstanceState to package-private like its peer StoragePaths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
The bounds check in restore() leans on OptionsActivity.paneCount(), but the fake screen computed tabClasses.length itself, so a wrong count stayed green in the suite and crashed on rotate. Both sides now go through a static tabCount(), and a test pins it against paneIndexOf(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OptionsActivity rebuilt its state from the launching intent on every recreation. A rotation, or a low-memory kill while the user was off in Help or the file chooser, silently reverted every unsaved edit and handed the stale map back on Back. It now saves the option map and the open tab into the instance bundle, through the same hooks
HTTrackActivityuses. The visible tab is flushed first, because a tab's widgets only reach the map when the tab is left, and restore goes throughsetPane(), so the tablet detail pane comes back with it.The bundle also carries a versionCode stamp, and a mismatch refuses the whole bundle, which is what
HTTrackActivityalready does. A Play update renumbersR.id, so an older bundle's map keys and pane index name other fields and other tabs, andfinish()would hand that scrambled map back to be written into the project profile.Save and restore now live in
OptionsInstanceStatebehind two seams, aStoreover theBundleand aScreenover the activity, so a plain JUnit test runs both directions instead of matching source text. Dropping the map write, swapping the map and pane keys, or dropping the version guard each fails it.Closes #129