Show Apple's terms of service in the wizard, instead of stopping at them - #121
Merged
Conversation
The CLI has been able to do this since the iCloud route landed; the window could not, and stopped at a delegate error with no way past it. Apple takes acceptance on one of its own devices or at icloud.com and nowhere else, so somebody with neither - which is everyone this project is for - was stuck. Same trigger as the CLI: a MobileMeDelegateError, because which error value means "terms pending" is not established, so this looks rather than assumes. Each document is shown whole through the renderer the CLI already uses, and acceptance is sent only for documents that were accepted. complete_login() follows, without which the account is left at AUTHENTICATED - readable, and unusable for everything after it. **One deliberate difference from the CLI, which asks before fetching.** It asks because it is about to page a document at a terminal and cannot take that back. Here the fetch is what decides whether there is anything to show, so asking first would mean offering to look and then reporting there was nothing. A delegate failure with no terms pending reports what Apple said, unchanged, rather than describing the check. Rejecting closes the exporter. That is the one exception to _load's rule that nothing closes the window from here, and it is deliberate: every other failure leaves the button there because trying again might work, and this one cannot - the same document is waiting on the next attempt. Its docstring now says so rather than contradicting the code. **Building the dialog and showing it are two functions, so it can be tested.** grab_set plus wait_window hand control to a nested event loop and a window manager; a test that schedules a click into that deadlocks rather than failing, which is what the first version of this did. Everything worth asserting is in the widgets, and all of it is unreachable once those two lines have run. For the same reason the keyboard bindings are asserted with widget.bind rather than pressed. A Toplevel under a withdrawn root holds focus from nobody, so event_generate is dropped in silence and "press Return, assert nothing happened" passes just as well when the key never arrived. It passed once here and failed on the next run, which is how it was noticed. Fixed-pitch, because terms.render underlines headings with a dash row as long as the heading. Proportional gets every rule wrong, and a contract that looks broken invites the conclusion that it has been tampered with. 28 tests: 13 for the flow with the dialog stubbed, needing no display, and 15 driving the real window. Verified by breaking seven things on purpose - an unanswered dialog counting as agreement, dropping complete_login, sending acceptance before asking, an editable terms box, Return bound to Accept, Reject leaving the window open, and Reject routed through the "please file a bug" handler - each of which reddens the tests that exist for it. **Not verified against Apple.** The trigger needs an account with terms actually pending, which cannot be produced on demand and which nobody here has. The flow tests drive a fake account. CONTRIBUTING said two files need Tk. It was already three before this, and is now four, so that is a table rather than a sentence - plus the two traps above, which are not worth anybody rediscovering. Co-Authored-By: Claude Opus 5 <noreply@anthropic.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
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.
The CLI has handled pending iCloud terms since the iCloud route landed. The window could not: it stopped at a
MobileMeDelegateErrorwith no way past it. Apple takes acceptance on one of its own devices or at icloud.com and nowhere else, so somebody with neither — which is everyone this project is for — was stuck.What it does
Same trigger as the CLI. Each document is rendered through
exporter.terms, which the CLI already uses, and shown in a modal with Accept and Reject. Acceptance is sent only for documents that were accepted, thencomplete_login()runs — without which the account is left atAUTHENTICATED: readable, and unusable for everything after it.Reject closes the exporter. That is the one exception to
_load's rule that nothing closes the window from here, and it is deliberate — every other failure leaves the button there because trying again might work, and this one cannot, since the same document is waiting on the next attempt. It says nothing was sent, that the account is unchanged, and where else terms can be accepted.One deliberate difference from the CLI
The CLI asks "Fetch the terms of service?" first, because it is about to page a document at a terminal and cannot take that back. Here the fetch is what decides whether there is anything to show, so asking first would mean offering to look and then reporting there was nothing. A delegate failure with no terms pending reports what Apple said, unchanged, rather than describing the check.
Testing
28 tests — 13 for the flow with the dialog stubbed, needing no display, and 15 driving the real window.
Building the dialog and showing it are separate functions so that it can be tested at all.
grab_setpluswait_windowhand control to a nested event loop and a window manager, and a test that schedules a click into that deadlocks rather than failing — which is what the first version of this did.For the same reason keyboard bindings are asserted with
widget.bindrather than pressed: aToplevelunder a withdrawn root holds focus from nobody, soevent_generateis dropped in silence and "press Return, assert nothing happened" passes just as well when the key never arrived. That one passed once and failed on the next run, which is how it was noticed.Verified by breaking seven things on purpose, each of which reddens the tests that exist for it:
complete_loginNot verified against Apple. The trigger needs an account with terms actually pending, which cannot be produced on demand and which nobody here has. The flow tests drive a fake account.
Also
CONTRIBUTING.mdsaid two files need Tk. It was already three before this and is now four, so that became a table, along with the two traps above.PR description summarised by Claude Code.