From 08b74768cc859e1ebef20d2f0d8a8dc534dd3ade Mon Sep 17 00:00:00 2001 From: Patrick Stel Date: Sun, 6 Sep 2026 15:19:56 +0200 Subject: [PATCH] Match a favourite by the page it is, not by the string it was written as MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue 80's reporter, on build-afa4ce8, within the hour: "pressing 8 again mostly doesn't show anything on favourited site. even when app shows unkept this page sometimes the page still shows in favourites." Both halves are real, neither is new, and keeping an address by typing it is simply the first thing that made them easy to hit. Favourites were matched by exact string. He kept instagram.com/reel by typing it, and Instagram's own address for that page ends in a slash — two strings, so pressing 8 on the page did not find his favourite, it added a second one. Both tiles are named after the same site, so what he saw was a page he had just been told was removed, still sitting in his favourites, because it was a different entry that looked identical. Store.SameKey folds a trailing slash on the path and only that. A query and a fragment stay significant: two addresses that differ there are two pages as often as they are one, and a favourite is an explicit act nobody should have quietly widened for them. The fold has a floor at the scheme's own //, or every site on earth would be one favourite. Init heals a file that already holds both spellings, the way #53's generated pages and waypoints are healed — his set has one now, and a fix that only stopped new ones would leave him deleting the old by hand from a page that cannot tell them apart. Key 8 was also reading a cache. _cachedUrl is refreshed by UpdateStatus, which runs when a load finishes, and a site that moves between reels without finishing another load leaves it several pages behind the screen — so 8 kept, or removed, a page the user was not looking at. It reads the engine now, as everything per-site already did. The bar's star reads the same cache, so it is refreshed on the tick while the bar is up: a few seconds at a time, and it leaves a flash alone. And the likeliest reading of "doesn't show anything": pressing 8 on the start screen did nothing, silently. There is no page there to keep, which is a reason to decline and no reason to say nothing — and the start screen, where the tiles are, is where somebody goes to get rid of one. It says so now, and points at "Keep an address...". tools/startpage/run.sh covers his case exactly, the fold's limits, the scheme floor, and the heal. --- docs/INTERNALS.md | 39 ++++++++++++++- src/common/Store.cs | 97 +++++++++++++++++++++++++++++++++++++- src/elm/BrowserApp.cs | 52 ++++++++++++++++++-- src/nui/NuiBrowserApp.cs | 44 +++++++++++++++-- tools/startpage/Program.cs | 49 +++++++++++++++++++ 5 files changed, 273 insertions(+), 8 deletions(-) diff --git a/docs/INTERNALS.md b/docs/INTERNALS.md index 22d2d45..c26d5ee 100644 --- a/docs/INTERNALS.md +++ b/docs/INTERNALS.md @@ -1607,6 +1607,40 @@ the two it just did — from a sofa, "kept" and "removed" are the same screen otherwise. And the tile's name is the site's own name (`SiteRules.KeyFor`), because there is no page title to take from a page nobody opened. +**And then it found two older faults, within the hour.** His report on +`build-afa4ce8`: "pressing 8 again mostly doesn't show anything on favourited +site. even when app shows unkept this page sometimes the page still shows in +favourites." Both halves are real and neither was new — keeping an address by +typing it is simply the first thing that made them easy to hit. + +- **Favourites were matched by exact string.** He kept `instagram.com/reel` by + typing it; Instagram's own address for that page ends in a slash. Two strings, + so pressing `8` on the page did not find his favourite — it added a second one, + and both tiles are named after the same site. What he saw was a page he had just + been told was removed, still sitting in his favourites, because it was a + different entry that looked identical. `Store.SameKey` now folds a trailing + slash on the path, and **only** that: a query and a fragment stay significant, + because two addresses that differ there are two pages as often as they are one, + and a favourite is an explicit act nobody should have quietly widened for them. + The fold has a floor at the scheme's own `//`, or every site on earth would be + one favourite. `Init` heals a file that already has both spellings in it, the + same way #53's generated pages and waypoints are healed, because his set has one + now and a fix that only stopped new ones would leave him deleting the old by + hand from a page that cannot tell them apart. +- **Key `8` was reading a cache.** `_cachedUrl` is refreshed by `UpdateStatus`, + which runs when a load *finishes* — and a site that moves between reels without + finishing another load leaves it several pages behind what is on the screen. So + `8` kept, or removed, a page the user was not looking at. It reads the engine + now, as everything per-site already did. The bar's star was reading the same + cache, so it is refreshed on the tick while the bar is up: a few seconds at a + time, and it leaves a flash alone. + +A third thing, smaller and the likeliest reading of "doesn't show anything": +**pressing `8` on the start screen did nothing, silently.** There is no page there +to keep, which is a fine reason to decline and no reason at all to say nothing — +and the start screen, where the tiles are, is exactly where somebody goes to get +rid of one. It now says so, and points at *Keep an address…*. + ## 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 @@ -2550,7 +2584,10 @@ the one its report has to come from. The state is: pointer keeps up on Instagram and Spotify now. If it does not, the `pointer` line on the report says which of the two is drawing it, and the answer that would matter is "drawn by Overscan and still slow", because that is the one - saying the lag was never our script. + saying the lag was never our script. **His first report on it was about #80**, + within the hour, and found two older faults behind the new feature — exact-string + favourite matching and key `8` reading a stale cache. See *Keeping an address you + cannot land on* above; both are fixed and his files are healed on load. Five things about that set are settled and should not be re-derived: **key `5` is his, not ours** — the engine's overlay path is the only one that gives him a diff --git a/src/common/Store.cs b/src/common/Store.cs index 54900f7..d492772 100644 --- a/src/common/Store.cs +++ b/src/common/Store.cs @@ -66,6 +66,15 @@ public static void Init(string dataDirectory) DiagLog.Add("store: dropped " + passed + " sign-in page(s) from history"); } + // And the same page kept twice under two spellings of its address — + // issue #80. See SameKey for how somebody ends up with two. + int twice = DropDuplicates(Favourites, "favourites.tsv") + + DropDuplicates(History, "history.tsv"); + if (twice > 0) + { + DiagLog.Add("store: dropped " + twice + " duplicate page(s)"); + } + DiagLog.Add("store: " + Favourites.Count + " favourites, " + History.Count + " history, " + Settings.Count + " settings"); } @@ -293,11 +302,62 @@ public static void Set(string key, bool value) Set(key, value ? "1" : "0"); } + /// + /// The two spellings of one page, reduced to one. + /// + /// Favourites were matched by exact string, and issue #80's reporter found + /// what that costs the moment there are two ways to write the same address: + /// he kept instagram.com/reel by typing it, and Instagram's own URL + /// for that page is instagram.com/reel/. So pressing 8 there did not + /// find his favourite — it added a second one — and both tiles are named + /// after the same site, so what he saw was a page he had just been told was + /// removed, still sitting in his favourites. + /// + /// Only the trailing slash is folded, and only on the path. A query and a + /// fragment stay significant, because two addresses that differ there are + /// two pages as often as they are one, and a favourite is an explicit act + /// that nobody should have quietly widened for them. + /// + private static string SameKey(string url) + { + if (string.IsNullOrEmpty(url)) + { + return string.Empty; + } + + int cut = url.Length; + int query = url.IndexOf('?'); + if (query >= 0) + { + cut = query; + } + + int fragment = url.IndexOf('#'); + if (fragment >= 0 && fragment < cut) + { + cut = fragment; + } + + string head = url.Substring(0, cut); + + // Never into the "//" of the scheme: "https://" must not become + // "https:/", which would fold every site on earth onto one key. + int authority = head.IndexOf("://", StringComparison.Ordinal); + int floor = authority < 0 ? 0 : authority + 3; + while (head.Length > floor && head[head.Length - 1] == '/') + { + head = head.Substring(0, head.Length - 1); + } + + return head + url.Substring(cut); + } + private static int IndexOf(List list, string url) { + string key = SameKey(url); for (int i = 0; i < list.Count; i++) { - if (string.Equals(list[i].Url, url, StringComparison.OrdinalIgnoreCase)) + if (string.Equals(SameKey(list[i].Url), key, StringComparison.OrdinalIgnoreCase)) { return i; } @@ -306,6 +366,41 @@ private static int IndexOf(List list, string url) return -1; } + /// + /// Removes entries that are the same page written two ways, keeping the + /// first — which in both files is the more recent. Heals a file an earlier + /// build wrote, the same way the generated pages and the sign-in waypoints + /// are healed: issue #80's reporter has look-alike favourites on his set + /// now, and a fix that only stops new ones appearing would leave him + /// deleting the old ones by hand from a page that cannot tell them apart. + /// + private static int DropDuplicates(List list, string fileName) + { + var seen = new List(); + int dropped = 0; + for (int i = 0; i < list.Count; i++) + { + string key = SameKey(list[i].Url).ToLowerInvariant(); + if (seen.Contains(key)) + { + list.RemoveAt(i); + i--; + dropped++; + } + else + { + seen.Add(key); + } + } + + if (dropped > 0) + { + Save(fileName, list); + } + + return dropped; + } + /// Removes generated pages a previous build let in, and saves if any were. private static int DropGenerated(List list, string fileName) { diff --git a/src/elm/BrowserApp.cs b/src/elm/BrowserApp.cs index 1a5ccc6..ac626c3 100644 --- a/src/elm/BrowserApp.cs +++ b/src/elm/BrowserApp.cs @@ -699,6 +699,16 @@ private bool OnTick() _cursor.FlushPending(); } + // The bar is read from a cache that only a finished load refreshes, and + // a site that moves between pages without finishing one (a reel, a + // playlist) would leave the address and the star showing the page + // before. Only while the bar is up, which is a few seconds at a time, + // and UpdateStatus leaves a flash alone. + if (_chromeVisible) + { + UpdateStatus(); + } + // The menu is on the list because arrowing through it is pointer keys, // which no longer keep the bar alive; without this it would time out // underneath an open menu. @@ -1778,16 +1788,52 @@ private string PageUrl() } } + /// + /// Keeps the page that is open, or takes it out again. + /// + /// Reads the address from the engine rather than from _cachedUrl, + /// and issue #80's reporter is why. That cache is refreshed by + /// UpdateStatus, which runs when a load finishes — and a site like + /// Instagram moves between reels without ever finishing another load, so + /// the cache could be several pages behind what was on the screen. Pressing + /// 8 then kept, or removed, a page the user was not looking at. + /// + /// The start screen has no page of its own to keep, and said so by doing + /// nothing at all, which from a sofa is indistinguishable from a key that + /// does not work — and the start screen, where the tiles are, is exactly + /// where somebody goes to get rid of one. + /// private void ToggleFavourite() { - if (_atHome) + string url = PageUrl(); + if (string.IsNullOrEmpty(url) || url == "-") { + Flash("Open a page first, or use \u201cKeep an address\u2026\u201d in the menu"); return; } - bool kept = Store.ToggleFavourite(_cachedUrl, _cachedTitle); - DiagLog.Add((kept ? "kept " : "removed ") + _cachedUrl); + bool kept = Store.ToggleFavourite(url, PageTitle()); + DiagLog.Add((kept ? "kept " : "removed ") + url); Flash(kept ? "Kept this page" : "Removed from favourites"); + + // The star on the bar is read from the same cache, so it would + // otherwise keep showing the answer from before the press. + _cachedUrl = url; + } + + /// The page's title now, not as of the last status refresh. + private string PageTitle() + { + try + { + string title = _web == null ? null : _web.Title; + return string.IsNullOrEmpty(title) ? _cachedTitle : title; + } + catch (Exception ex) + { + DiagLog.Add("reading the page title failed: " + ex.Message); + return _cachedTitle; + } } /// diff --git a/src/nui/NuiBrowserApp.cs b/src/nui/NuiBrowserApp.cs index 393acd9..9fdcf45 100644 --- a/src/nui/NuiBrowserApp.cs +++ b/src/nui/NuiBrowserApp.cs @@ -890,6 +890,16 @@ private bool OnTick() CheckSomethingLoaded(); + // The bar is read from a cache that only a finished load refreshes, and + // a site that moves between pages without finishing one (a reel, a + // playlist) would leave the address and the star showing the page + // before. Only while the bar is up, which is a few seconds at a time, + // and UpdateStatus leaves a flash alone. + if (_chromeVisible) + { + UpdateStatus(); + } + // The menu joins the list now that the pointer no longer keeps the bar // alive: arrowing through the menu is pointer keys, and without this the // chrome would time out underneath an open menu. @@ -2330,16 +2340,44 @@ private string PageUrl() return _web == null || _atHome ? null : SafeUrl(); } + /// + /// Keeps the page that is open, or takes it out again. + /// + /// Reads the address from the engine rather than from _cachedUrl, + /// and issue #80's reporter is why. That cache is refreshed by + /// UpdateStatus, which runs when a load finishes — and a site like + /// Instagram moves between reels without ever finishing another load, so + /// the cache could be several pages behind what was on the screen. Pressing + /// 8 then kept, or removed, a page the user was not looking at. + /// + /// The start screen has no page of its own to keep, and said so by doing + /// nothing at all, which from a sofa is indistinguishable from a key that + /// does not work — and the start screen, where the tiles are, is exactly + /// where somebody goes to get rid of one. + /// private void ToggleFavourite() { - if (_atHome) + string url = PageUrl(); + if (string.IsNullOrEmpty(url) || url == "-") { + Flash("Open a page first, or use \u201cKeep an address\u2026\u201d in the menu"); return; } - bool kept = Store.ToggleFavourite(_cachedUrl, _cachedTitle); - DiagLog.Add((kept ? "kept " : "removed ") + _cachedUrl); + bool kept = Store.ToggleFavourite(url, PageTitle()); + DiagLog.Add((kept ? "kept " : "removed ") + url); Flash(kept ? "Kept this page" : "Removed from favourites"); + + // The star on the bar is read from the same cache, so it would + // otherwise keep showing the answer from before the press. + _cachedUrl = url; + } + + /// The page's title now, not as of the last status refresh. + private string PageTitle() + { + string title = SafeTitle(); + return string.IsNullOrEmpty(title) ? _cachedTitle : title; } /// diff --git a/tools/startpage/Program.cs b/tools/startpage/Program.cs index 486cb72..3734ee5 100644 --- a/tools/startpage/Program.cs +++ b/tools/startpage/Program.cs @@ -155,6 +155,55 @@ private static int Main(string[] args) Check(DiagLog.Lines.Exists(l => l.Contains("dropped 2 sign-in")), "the log says how many: " + string.Join(" | ", DiagLog.Lines)); + // 5b. Two spellings of one page are one favourite (issue #80). His + // case exactly: kept by typing instagram.com/reel, pressed 8 on the + // page, whose own address ends in a slash. + string dupDir = Path.Combine(dir, "duplicates"); + Directory.CreateDirectory(dupDir); + Store.Init(dupDir); + Check(Store.ToggleFavourite("https://www.instagram.com/reel", "instagram.com"), + "an address typed without a trailing slash is kept"); + Check(Store.IsFavourite("https://www.instagram.com/reel/"), + "and the page's own address, with one, is the same favourite"); + Check(!Store.ToggleFavourite("https://www.instagram.com/reel/", "Instagram") && + Store.AllFavourites.Count == 0, + "so pressing 8 there removes it instead of adding a second"); + + // The fold is the trailing slash and nothing else. A query and a + // fragment still tell two pages apart, because they usually do. + Store.ToggleFavourite("https://example.com/a?x=1", "A"); + Check(!Store.IsFavourite("https://example.com/a?x=2"), + "a different query is a different page"); + Check(!Store.IsFavourite("https://example.com/b"), + "and so is a different path"); + Check(Store.IsFavourite("https://example.com/a/?x=1"), + "while a slash before the query is not"); + + // The floor: nothing may fold into the scheme's own slashes, or every + // site on earth would be one favourite. + Store.ToggleFavourite("https://one.test/", "one"); + Check(!Store.IsFavourite("https://two.test/"), "two sites are never one favourite"); + + // 5c. A file an earlier build wrote, with both spellings in it, is + // healed on load — his set has one now. + string dupHealDir = Path.Combine(dir, "duplicates-heal"); + Directory.CreateDirectory(dupHealDir); + File.WriteAllLines(Path.Combine(dupHealDir, "favourites.tsv"), new[] + { + "https://www.instagram.com/reel\tinstagram.com", + "https://www.instagram.com/reel/\tInstagram", + "https://open.spotify.com/\tSpotify", + }); + DiagLog.Lines.Clear(); + Store.Init(dupHealDir); + Check(Store.AllFavourites.Count == 2, "the look-alike favourite is dropped on load"); + Check(Store.AllFavourites[0].Url == "https://www.instagram.com/reel", + "keeping the first, which is the more recent"); + Check(File.ReadAllLines(Path.Combine(dupHealDir, "favourites.tsv")).Length == 2, + "and the file is written back clean"); + Check(DiagLog.Lines.Exists(l => l.Contains("dropped 1 duplicate")), + "the log says how many: " + string.Join(" | ", DiagLog.Lines)); + // 6. Where the browser opens — three states, not two (issue #79). // The one that has to keep working is the upgrade: an install made // before this existed has an address and no mode, and its start page