diff --git a/docs/INTERNALS.md b/docs/INTERNALS.md index e394aad..b9b5571 100644 --- a/docs/INTERNALS.md +++ b/docs/INTERNALS.md @@ -1721,6 +1721,43 @@ by `HomePage`, returned by `linkAt` in front of the address). The same page is v often a favourite and a recent visit both, and removing it from the list the pointer was not on is indistinguishable, from a sofa, from a key that did nothing. +### Opening a favourite never told the app it had left the start screen + +His fourth report, and the best one: + +> 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` — 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 +(which is the whole reason the start screen is a page and not 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, and everything keyed to it went wrong in a different way: + +- 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 (`PageUrl` + returns null there, and `SiteRules.SetImages(null, …)` correctly declines); +- key `8` looked for a tile under the pointer and found none; +- the address bar said `start screen` over a page that was plainly not it; +- and turning images back on flashed them for a single frame before the reload + put the site's own rule back — which is the symptom that proves the diagnosis, + because it is the global setting and the site rule disagreeing out loud. + +The fix is one line in the right place, and the rule behind it is the general one: +**state that describes the page must be derived from the page, not from who asked +for it.** `NoteShowing` is called at every load boundary and sets the flag from +`Store.IsGenerated`, so a navigation nobody in this app initiated still moves it. +`ShowHome` and `Navigate` still set it optimistically, since the bar should not lag +a press, but they are no longer the authority. + +This had been wrong since the start screen existed. It cost nothing until #74 gave +the app something important to decide with it, which is the usual shape: the bug +ships years before the feature that makes it reachable. + ## Settings that belong to a site, not to the browser Issues #74 and #75, from the same reporter, four days apart. Both are the same diff --git a/src/elm/BrowserApp.cs b/src/elm/BrowserApp.cs index 0f67fe4..a5ffb2e 100644 --- a/src/elm/BrowserApp.cs +++ b/src/elm/BrowserApp.cs @@ -1208,6 +1208,7 @@ private void ConfigureWebView() ApplyViewportFix(); ReportMetrics(); Store.RecordVisit(_web.Url, _web.Title); + NoteShowing(_web.Url); // Backstop for UrlChanged, which is where a site's rules normally // land. A page that somehow arrives without one still gets them, @@ -1235,6 +1236,10 @@ private void ConfigureWebView() // early enough for the images and one reload late for the identity. string arrived = e.GetAsString(); _urlLabel.Text = Markup(arrived); + + // Before the site rules, which ask whether this page is on a site + // at all — and "the start screen" is the answer that means no. + NoteShowing(arrived); ApplySiteRules(arrived, true); UpdateStatus(); }; @@ -1769,6 +1774,34 @@ private void Reload(string why) } } + /// + /// Records what the view has actually got on it, which is the only thing + /// that may decide whether we are on the start screen. + /// + /// + /// _atHome used to be set by ShowHome and cleared by + /// Navigate — that is, by whoever *asked* for a page, which misses + /// every navigation the app does not perform itself. **Opening a favourite + /// is one of those**: a tile is an ordinary link, so the engine follows it + /// and nothing tells the app it left. So the browser sat on Instagram still + /// believing it was showing the start screen, and issue #80's reporter found + /// what that costs once things started keying off it: the images and + /// identity switches wrote the browser-wide setting instead of the site's + /// (because "no site" is what the start screen is), key 8 looked for a tile + /// under the pointer, the address bar said "start screen", and turning + /// images back on flashed them for one frame before the site's own rule + /// reloaded them away again. Every one of those is the same wrong bit. + /// + /// A page is the start screen if it *is* the start screen. Both shapes of + /// that are , and this is called at every + /// load boundary, so a navigation nobody in this app initiated still moves + /// the flag. + /// + private void NoteShowing(string url) + { + _atHome = Store.IsGenerated(url); + } + /// /// The address the page is on, or null when it is one of ours. Everything /// per-site keys off this, and "no site" is a real answer rather than a diff --git a/src/nui/NuiBrowserApp.cs b/src/nui/NuiBrowserApp.cs index 063be9a..41c8d24 100644 --- a/src/nui/NuiBrowserApp.cs +++ b/src/nui/NuiBrowserApp.cs @@ -484,7 +484,10 @@ private bool TryStartEngine() // The engine reports the destination here, which is where a // site's own settings go on the view — see ApplySiteRules for // why that is early enough for the images and one reload late - // for the identity. + // for the identity. NoteShowing comes first: the rules ask + // whether this page is on a site at all, and "the start screen" + // is the answer that means no. + NoteShowing(SafeUrl()); ApplySiteRules(SafeUrl(), true); // The engine answered, so the view is not the dead kind. @@ -521,6 +524,7 @@ private bool TryStartEngine() Probe(); ApplyViewportFix(); Store.RecordVisit(SafeUrl(), SafeTitle()); + NoteShowing(SafeUrl()); // Backstop, for a load whose start we somehow missed. Normally // a no-op: the site is already the applied one by now. @@ -2330,6 +2334,34 @@ private void Reload(string why) } } + /// + /// Records what the view has actually got on it, which is the only thing + /// that may decide whether we are on the start screen. + /// + /// + /// _atHome used to be set by ShowHome and cleared by + /// Navigate — that is, by whoever *asked* for a page, which misses + /// every navigation the app does not perform itself. **Opening a favourite + /// is one of those**: a tile is an ordinary link, so the engine follows it + /// and nothing tells the app it left. So the browser sat on Instagram still + /// believing it was showing the start screen, and issue #80's reporter found + /// what that costs once things started keying off it: the images and + /// identity switches wrote the browser-wide setting instead of the site's + /// (because "no site" is what the start screen is), key 8 looked for a tile + /// under the pointer, the address bar said "start screen", and turning + /// images back on flashed them for one frame before the site's own rule + /// reloaded them away again. Every one of those is the same wrong bit. + /// + /// A page is the start screen if it *is* the start screen. Both shapes of + /// that are , and this is called at every + /// load boundary, so a navigation nobody in this app initiated still moves + /// the flag. + /// + private void NoteShowing(string url) + { + _atHome = Store.IsGenerated(url); + } + /// /// The address the page is on, or null when it is one of ours. Everything /// per-site keys off this, and "no site" is a real answer rather than a