Let the page decide whether we are on the start screen - #89
Merged
Merged
Conversation
Issue 80's reporter: "when i open site that had images turned on previously from favourites and turn images off it turns off images globally instead for that site ... if visit website by typing url turning images on/off changes setting only for that site". _atHome was set by ShowHome and cleared by Navigate — by whoever asked for a page. That misses every navigation this app does not perform itself, and opening a favourite is one of them: a tile is an ordinary link, which is the whole reason the start screen is a page rather than a native screen, so the engine follows it and nothing tells the app it left. The browser then sat on Instagram still believing it was showing its own start screen. One wrong bit, four faces. The images and identity switches wrote the browser-wide setting instead of the site's, because "no site" is exactly what the start screen is. Key 8 looked for a tile under the pointer and found none. The address bar said "start screen" over a page that plainly was not it. And turning images back on flashed them for one frame before the reload put the site's own rule back — the symptom that proves the diagnosis, since it is the global setting and the site rule disagreeing out loud. NoteShowing is called at every load boundary and sets the flag from Store.IsGenerated, so a navigation nobody here initiated still moves it. ShowHome and Navigate still set it optimistically, because the bar should not lag a press, but they are no longer the authority. The rule: state that describes the page must be derived from the page, not from who asked for it. This had been wrong since the start screen existed and cost nothing until #74 gave the app something important to decide with it.
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.
His fourth report, and the best one:
One wrong bit
_atHomewas set byShowHomeand cleared byNavigate— that is, by whoever asked for a page. That misses every navigation this app does not perform itself, and opening a favourite is one of them: a tile is an ordinary link (the whole reason the start screen is a page rather than a native screen), so the engine follows it and nothing tells the app it left.The browser then sat on Instagram still believing it was showing its own start screen. Everything keyed to that went wrong in a different way:
PageUrl()returned null andSiteRules.SetImages(null, …)correctly declined;8looked for a tile under the pointer and found none;start screenover a page that plainly was not it;His control case — typing the URL — worked, because
Navigate()is the one path that did clear the flag.The fix, and the rule behind it
NoteShowingis called at every load boundary and sets the flag fromStore.IsGenerated, so a navigation nobody in this app initiated still moves it.ShowHomeandNavigatestill set it optimistically (the bar should not lag a press) but are no longer the authority.State that describes the page must be derived from the page, not from who asked for it.
This had been wrong since the start screen existed. It cost nothing until #74 gave the app something important to decide with it — the usual shape: the bug ships years before the feature that makes it reachable.
Checks
./build.sh all(five packages, 0 warnings) andtools/startpage/run.shgreen. Nosrc/commonchange this time — the decision it now defers to,Store.IsGenerated, is the one the start-page harness has covered since #53.