From ddc1a02c642d5900fd752b0024edf983554d2627 Mon Sep 17 00:00:00 2001 From: Patrick Stel Date: Sun, 6 Sep 2026 17:34:14 +0200 Subject: [PATCH] Give each favourites gesture one meaning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue 80's third report in one afternoon: "app allows two duplicates url to be added when used both 8 button & keep page menu together, if we point at the tile added using kept menu and press 8 it duplicates that site." Pointing at a tile and pressing 8 added a second copy of it — in the build shipped an hour earlier to let him remove tiles by pointing at them. Three reports, three fixes, each one a better rule for matching addresses. The matching rule was never the fault. The fault is that 8 meant two opposite things depending on state the screen did not show. On a page it kept or removed depending on an address the tile never displayed. On the start screen it kept or removed depending on which of two grids the pointer was over — and those grids are the same tiles, drawn the same way, one above the other, very often holding the same page. Point at something already kept, press the key that removes things, get another copy. Every fix that preserved the toggle bought one more shape of the same report. So the gestures are separated by meaning rather than by state: 8 on a page keep this page, or drop it 8 on a tile get rid of this tile — never adds Keep an address... keep this address — never removes, never a second copy 8 stays a toggle on a page because you are looking at the thing itself, so both outcomes are legible before the press. Nowhere else is that true. And a menu row with the word keep in it must not sometimes delete. A tile now carries which grid it is in (data-kind, written by HomePage and returned by linkAt in front of the address), because the same page is very often a favourite and a recent visit both, and removing it from the list the pointer was not on looks exactly like a key that did nothing. 8 on a recent tile forgets that visit. startpage holds the property rather than the cases: no sequence of presses makes a second tile for something already on the screen. linkAt verified in desktop chromium against both grids carrying the same URL; cdpharness re-run. --- README.md | 22 ++++++-- docs/INTERNALS.md | 44 +++++++++++++++ src/common/HomePage.cs | 16 ++++-- src/common/PageScript.cs | 13 ++++- src/common/Store.cs | 48 ++++++++++++++++ src/elm/BrowserApp.cs | 112 +++++++++++++++++++++++-------------- src/nui/NuiBrowserApp.cs | 108 ++++++++++++++++++++++------------- tools/startpage/Program.cs | 44 +++++++++++++++ 8 files changed, 314 insertions(+), 93 deletions(-) diff --git a/README.md b/README.md index a57b2ac..df7ef31 100644 --- a/README.md +++ b/README.md @@ -99,12 +99,24 @@ Every digit is spoken for, so these live in the menu only (**hold OK**): | **Pointer style** | Who draws the pointer — Overscan (keeps up) or the page (an arrow) | | **Ad blocking on/off** | 2025+ package only | -### Keeping a page you can't land on +### Favourites -**Keep an address…** in the menu is for URLs that redirect. `https://www.instagram.com/reel` -sends you to one particular reel, so pressing `8` there would save that clip -for ever; type the address instead and the tile is the address. It's prefilled -with wherever you are, so it's usually a matter of deleting the end of it. +Three gestures, one meaning each: + +| | | +| --- | --- | +| **8** on a page | Keep it, or drop it | +| **8** on a tile | Remove that tile — point at it on the start screen | +| **Keep an address…** | Keep a URL you can't land on | + +**Keep an address…** is for URLs that redirect. `https://www.instagram.com/reel` +sends you to one particular reel, so pressing `8` there would save that clip for +ever; type the address instead and the tile is the address. It's prefilled with +wherever you are, so it's usually a matter of deleting the end of it. + +To get rid of anything, point at its tile on the start screen and press **8** — +you never have to remember how it got there or what it was called. That works on +**Recent** tiles too, if you'd rather a page wasn't listed. ### Where it opens diff --git a/docs/INTERNALS.md b/docs/INTERNALS.md index fff199d..335adbc 100644 --- a/docs/INTERNALS.md +++ b/docs/INTERNALS.md @@ -1677,6 +1677,50 @@ Shipped in `build-a77a661`. The rule this leaves behind: **anything the app will act on by name must be displayed under that name, or be reachable without one.** Favourites were neither. +### One gesture, one meaning + +And then he reported it a third time, within the hour, and the third report is the +one that names the actual fault: + +> app allows two duplicates url to be added when used both 8 button & keep page +> menu together, if we point at the tile added using kept menu and press 8 it +> duplicates that site. + +Pointing at a tile and pressing `8` **added a second copy of it** — in the build +that had just been shipped to let him remove tiles by pointing at them. That is +three reports in one afternoon, and every one of them was fixed by a better rule +for matching addresses. The rule was never the problem. + +**The fault is that `8` meant two opposite things depending on state the screen did +not show.** On a page it kept-or-removed depending on an address the tile never +displayed. On the start screen it kept-or-removed depending on which of two grids +the pointer was over — and the two grids are the same tiles, drawn the same way, +one above the other, very often containing the same page. Point at something you +already kept, press the key that removes things, get another copy. Each fix that +preserved the toggle bought exactly one more shape of the same report. + +So the gestures are separated by meaning rather than by state, and each has one: + +| Gesture | Means | Never | +| --- | --- | --- | +| `8` on a page | keep this page, or drop it | — | +| `8` on a tile | get rid of this tile | adds | +| *Keep an address…* | keep this address | removes, or adds a second copy | + +`8` stays a toggle **on a page** because you are looking at the thing itself, so +both outcomes are legible before you press. Nowhere else is that true. And a menu +row with the word *keep* in it must not sometimes delete, which is the reading its +name promises and the one it did not have. + +There is now no sequence of presses that produces a second tile for something +already on the screen, and `tools/startpage/run.sh` holds that as a property +rather than as a list of cases. + +One supporting detail: a tile carries **which grid it is in** (`data-kind`, written +by `HomePage`, returned by `linkAt` in front of the address). The same page is very +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. + ## 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/common/HomePage.cs b/src/common/HomePage.cs index 75a2552..3ce40d6 100644 --- a/src/common/HomePage.cs +++ b/src/common/HomePage.cs @@ -61,13 +61,13 @@ public static string Build(IList favourites, IList history, } else { - AppendGrid(html, favourites, 12); + AppendGrid(html, favourites, 12, "fav"); } if (history.Count > 0) { html.Append("

Recent

"); - AppendGrid(html, history, 8); + AppendGrid(html, history, 8, "recent"); } // Four things that get somebody unstuck on their first evening, and @@ -76,6 +76,7 @@ public static string Build(IList favourites, IList history, html.Append(@"
0 type an address  ·  7 the remote card — every key, and what it is for  ·  9 back to this screen  ·  channel up/down scrolls, on every remote
+8 on a tile removes it  ·  on a page, keeps it here.
Move the pointer with the D-pad and press OK to click. On the keyboard, start makes what you typed the page this browser opens on; press it with nothing typed to get this screen back. @@ -83,13 +84,20 @@ to get this screen back. return html.ToString(); } - private static void AppendGrid(StringBuilder html, IList items, int limit) + /// + /// One grid of tiles. is written onto each tile so + /// that pressing 8 on one knows which list it is looking at: the same page + /// is very often in both grids, and taking it out of the wrong one is + /// indistinguishable, from a sofa, from a key that did nothing. + /// + private static void AppendGrid(StringBuilder html, IList items, int limit, string kind) { html.Append("
"); for (int i = 0; i < items.Count && i < limit; i++) { Bookmark item = items[i]; - html.Append("") + html.Append("") .Append("").Append(Escape(HostOf(item.Url))).Append("") .Append("").Append(Escape(item.Title)).Append("") .Append(""); diff --git a/src/common/PageScript.cs b/src/common/PageScript.cs index a40792a..1134581 100644 --- a/src/common/PageScript.cs +++ b/src/common/PageScript.cs @@ -220,10 +220,17 @@ removed by any spelling he could see. Pointing at the thing you want gone is var n = at(); for (var depth = 0; n && depth < 8; depth++) { if (n.tagName === 'A' && n.getAttribute && n.getAttribute('href')) { - /* .href is resolved against the page's base, which is what the app + /* Which list the tile is in, then the address. The same page is very + often a favourite *and* a recent visit, so a caller told only the + address cannot know which of the two tiles the pointer was on — and + taking a page out of the wrong list is indistinguishable, from a sofa, + from a key that did nothing. + + .href is resolved against the page's base, which is what the app stored; the attribute is the fallback for engines that do not give a - resolved one on a detached node. */ - return String(n.href || n.getAttribute('href')); + resolved one. */ + return String(n.getAttribute('data-kind') || 'link') + ' ' + + String(n.href || n.getAttribute('href')); } n = n.parentElement || n.parentNode; diff --git a/src/common/Store.cs b/src/common/Store.cs index 598cfa7..fd0c943 100644 --- a/src/common/Store.cs +++ b/src/common/Store.cs @@ -234,6 +234,54 @@ public static bool ToggleFavourite(string url, string title) return true; } + /// + /// Keeps a page, and says whether it was already kept. Never removes. + /// + /// The add-only half of , for "Keep an + /// address…" — a menu row with the word *keep* in it must not sometimes + /// delete, and the reporter on issue #80 spent an afternoon in the gap + /// between those two readings. + /// + public static bool Keep(string url, string title) + { + if (string.IsNullOrEmpty(url) || url == "-" || IsGenerated(url) || IndexOf(Favourites, url) >= 0) + { + return false; + } + + Favourites.Insert(0, new Bookmark(url, string.IsNullOrEmpty(title) ? url : title)); + Save("favourites.tsv", Favourites); + return true; + } + + /// Drops a favourite. True when there was one to drop. + public static bool RemoveFavourite(string url) + { + int at = IndexOf(Favourites, url); + if (at < 0) + { + return false; + } + + Favourites.RemoveAt(at); + Save("favourites.tsv", Favourites); + return true; + } + + /// Drops a visit. True when there was one to drop. + public static bool ForgetVisit(string url) + { + int at = IndexOf(History, url); + if (at < 0) + { + return false; + } + + History.RemoveAt(at); + Save("history.tsv", History); + return true; + } + public static void RecordVisit(string url, string title) { if (string.IsNullOrEmpty(url) || url == "-" || url.StartsWith("about:", StringComparison.Ordinal)) diff --git a/src/elm/BrowserApp.cs b/src/elm/BrowserApp.cs index 77014e0..0f67fe4 100644 --- a/src/elm/BrowserApp.cs +++ b/src/elm/BrowserApp.cs @@ -1275,7 +1275,7 @@ private void OnBridgeMessage(JavaScriptMessage message) break; case "tile": - ToggleTile(parts.Length > 1 ? parts[1] : null); + ForgetTile(parts.Length > 1 ? parts[1] : null); break; case "typed": @@ -1812,7 +1812,7 @@ private void ToggleFavourite() if (_atHome) { // On the start screen the answer is a tile, and the page has to be - // asked which one. It comes back over the bridge, into ToggleTile. + // asked which one. It comes back over the bridge, into ForgetTile. try { _web.Eval("try{window." + BridgeName + ".postMessage('tile\u0001'+String(window." + @@ -1843,55 +1843,74 @@ private void ToggleFavourite() } /// - /// Keeps or removes the favourite whose tile the pointer is on. + /// Removes the tile the pointer is on. On the start screen, that is all 8 + /// does — it never adds. + /// + /// + /// Issue #80 arrived three times in one afternoon, each time as a different + /// symptom of one thing: a gesture that meant two opposite things + /// depending on state the screen did not show. First 8 kept or removed + /// depending on an address the tile never displayed; then it kept or removed + /// depending on which of two identical-looking grids the pointer happened to + /// be over, so pointing at a page already kept and pressing 8 added a second + /// copy of it. Every fix that kept the toggle bought one more shape of the + /// same report. /// - /// The start screen is where the tiles are, so it is where somebody goes to - /// get rid of one — and until this existed there was no way to. Removing a - /// favourite meant being on its page and pressing 8, or typing its address - /// into "Keep an address…" exactly as it was stored, and issue #80's - /// reporter had two tiles reading identically, pointing at different pages, - /// stored under addresses the screen never showed him. Pointing at the thing - /// you want gone is the one gesture that cannot be spelled wrong. + /// So the gestures are separated by meaning instead, and each one has + /// exactly one: + /// + /// 8 on a page — keep this page, or drop it. You are looking + /// at the thing, so a toggle is honest. + /// 8 on a tile — get rid of this tile. Never adds; you can + /// see it is already there. + /// Keep an address… — keeps. A menu row with the word keep in + /// it must not sometimes delete. + /// + /// There is now no sequence of presses that makes a second tile for + /// something already on the screen. /// - /// A recent tile gets kept rather than removed, which is the same key doing - /// the same thing it does everywhere else. - /// - private void ToggleTile(string url) + /// The tile carries which grid it is in (data-kind, written by + /// ), because the same page is very often a favourite + /// and a recent visit both, and removing it from the list the pointer was + /// not on looks exactly like a key that did nothing. + /// + private void ForgetTile(string answer) { + string kind = string.Empty; + string url = answer ?? string.Empty; + int space = url.IndexOf(' '); + if (space > 0) + { + kind = url.Substring(0, space); + url = url.Substring(space + 1); + } + if (string.IsNullOrEmpty(url) || url == "null" || Store.IsGenerated(url)) { - Flash("Point at a tile first, then press 8"); + Flash("Point at a tile, then press 8 to remove it"); return; } - bool kept = Store.ToggleFavourite(url, TitleForTile(url)); - DiagLog.Add((kept ? "kept tile " : "removed tile ") + url); - Flash((kept ? "Kept " : "Removed ") + Urls.Readable(url)); + bool gone = kind == "recent" ? Store.ForgetVisit(url) : Store.RemoveFavourite(url); + if (!gone) + { + // Nothing was there to remove, which on this screen means the tile + // is not what we were told it is. Say so rather than reporting a + // removal that did not happen. + DiagLog.Add("tile not found in " + (kind == "recent" ? "history" : "favourites") + ": " + url); + Flash("That tile is not in the list any more"); + ShowHome(); + return; + } + + DiagLog.Add("removed " + kind + " tile " + url); + Flash("Removed " + Urls.Readable(url)); // Rebuilt, or the tile just removed is still on the screen — which is // exactly the "it did not work" this whole thread is about. ShowHome(); } - /// - /// The name to keep a tile under: whatever it was already called if we know - /// it, and the address otherwise. Never the bare host — two favourites on - /// one site would then be one name twice, which is how issue #80's reporter - /// ended up unable to tell his apart. - /// - private static string TitleForTile(string url) - { - foreach (Bookmark seen in Store.RecentHistory) - { - if (string.Equals(seen.Url, url, StringComparison.OrdinalIgnoreCase)) - { - return seen.Title; - } - } - - return Urls.Readable(url); - } - /// The page's title now, not as of the last status refresh. private string PageTitle() { @@ -1924,15 +1943,24 @@ private void KeepAddress(string text) { string url = Urls.Normalize(text); - // No title to take from a page nobody opened, so the site's own name is - // the honest one. The tiles are named by this. // The address, not the bare host. Two favourites on one site would // otherwise be one name twice, and the tiles are all somebody has to // tell them apart by — issue #80's follow-up. string title = Urls.Readable(url); - bool kept = Store.ToggleFavourite(url, title); - DiagLog.Add((kept ? "kept address " : "removed address ") + url); - Flash(kept ? "Kept " + title : "Removed " + title + " from favourites"); + + // Keeps. Never removes, and never a second copy of something already + // there: a row with the word keep in it that sometimes deletes is how + // this issue got to its third report. Removing is 8 on the tile. + if (Store.Keep(url, title)) + { + DiagLog.Add("kept address " + url); + Flash("Kept " + title); + } + else + { + DiagLog.Add("already kept: " + url); + Flash(title + " is already kept — press 8 on its tile to remove it"); + } } /// diff --git a/src/nui/NuiBrowserApp.cs b/src/nui/NuiBrowserApp.cs index 1428be9..063be9a 100644 --- a/src/nui/NuiBrowserApp.cs +++ b/src/nui/NuiBrowserApp.cs @@ -2366,7 +2366,7 @@ private void ToggleFavourite() _web.EvaluateJavaScript( "String(window." + PageScript.Namespace + " && window." + PageScript.Namespace + ".linkAt())", - result => ToggleTile(result)); + result => ForgetTile(result)); } catch (Exception ex) { @@ -2393,55 +2393,74 @@ private void ToggleFavourite() } /// - /// Keeps or removes the favourite whose tile the pointer is on. + /// Removes the tile the pointer is on. On the start screen, that is all 8 + /// does — it never adds. + /// + /// + /// Issue #80 arrived three times in one afternoon, each time as a different + /// symptom of one thing: a gesture that meant two opposite things + /// depending on state the screen did not show. First 8 kept or removed + /// depending on an address the tile never displayed; then it kept or removed + /// depending on which of two identical-looking grids the pointer happened to + /// be over, so pointing at a page already kept and pressing 8 added a second + /// copy of it. Every fix that kept the toggle bought one more shape of the + /// same report. /// - /// The start screen is where the tiles are, so it is where somebody goes to - /// get rid of one — and until this existed there was no way to. Removing a - /// favourite meant being on its page and pressing 8, or typing its address - /// into "Keep an address…" exactly as it was stored, and issue #80's - /// reporter had two tiles reading identically, pointing at different pages, - /// stored under addresses the screen never showed him. Pointing at the thing - /// you want gone is the one gesture that cannot be spelled wrong. + /// So the gestures are separated by meaning instead, and each one has + /// exactly one: + /// + /// 8 on a page — keep this page, or drop it. You are looking + /// at the thing, so a toggle is honest. + /// 8 on a tile — get rid of this tile. Never adds; you can + /// see it is already there. + /// Keep an address… — keeps. A menu row with the word keep in + /// it must not sometimes delete. + /// + /// There is now no sequence of presses that makes a second tile for + /// something already on the screen. /// - /// A recent tile gets kept rather than removed, which is the same key doing - /// the same thing it does everywhere else. - /// - private void ToggleTile(string url) + /// The tile carries which grid it is in (data-kind, written by + /// ), because the same page is very often a favourite + /// and a recent visit both, and removing it from the list the pointer was + /// not on looks exactly like a key that did nothing. + /// + private void ForgetTile(string answer) { + string kind = string.Empty; + string url = answer ?? string.Empty; + int space = url.IndexOf(' '); + if (space > 0) + { + kind = url.Substring(0, space); + url = url.Substring(space + 1); + } + if (string.IsNullOrEmpty(url) || url == "null" || Store.IsGenerated(url)) { - Flash("Point at a tile first, then press 8"); + Flash("Point at a tile, then press 8 to remove it"); return; } - bool kept = Store.ToggleFavourite(url, TitleForTile(url)); - DiagLog.Add((kept ? "kept tile " : "removed tile ") + url); - Flash((kept ? "Kept " : "Removed ") + Urls.Readable(url)); + bool gone = kind == "recent" ? Store.ForgetVisit(url) : Store.RemoveFavourite(url); + if (!gone) + { + // Nothing was there to remove, which on this screen means the tile + // is not what we were told it is. Say so rather than reporting a + // removal that did not happen. + DiagLog.Add("tile not found in " + (kind == "recent" ? "history" : "favourites") + ": " + url); + Flash("That tile is not in the list any more"); + ShowHome(); + return; + } + + DiagLog.Add("removed " + kind + " tile " + url); + Flash("Removed " + Urls.Readable(url)); // Rebuilt, or the tile just removed is still on the screen — which is // exactly the "it did not work" this whole thread is about. ShowHome(); } - /// - /// The name to keep a tile under: whatever it was already called if we know - /// it, and the address otherwise. Never the bare host — two favourites on - /// one site would then be one name twice, which is how issue #80's reporter - /// ended up unable to tell his apart. - /// - private static string TitleForTile(string url) - { - foreach (Bookmark seen in Store.RecentHistory) - { - if (string.Equals(seen.Url, url, StringComparison.OrdinalIgnoreCase)) - { - return seen.Title; - } - } - - return Urls.Readable(url); - } - /// The page's title now, not as of the last status refresh. private string PageTitle() { @@ -2470,9 +2489,20 @@ private void KeepAddress(string text) // otherwise be one name twice, and the tiles are all somebody has to // tell them apart by — issue #80's follow-up. string title = Urls.Readable(url); - bool kept = Store.ToggleFavourite(url, title); - DiagLog.Add((kept ? "kept address " : "removed address ") + url); - Flash(kept ? "Kept " + title : "Removed " + title + " from favourites"); + + // Keeps. Never removes, and never a second copy of something already + // there: a row with the word keep in it that sometimes deletes is how + // this issue got to its third report. Removing is 8 on the tile. + if (Store.Keep(url, title)) + { + DiagLog.Add("kept address " + url); + Flash("Kept " + title); + } + else + { + DiagLog.Add("already kept: " + url); + Flash(title + " is already kept — press 8 on its tile to remove it"); + } } /// diff --git a/tools/startpage/Program.cs b/tools/startpage/Program.cs index e4b70cb..6548e8b 100644 --- a/tools/startpage/Program.cs +++ b/tools/startpage/Program.cs @@ -219,6 +219,50 @@ private static int Main(string[] args) Check(DiagLog.Lines.Exists(l => l.Contains("dropped 1 duplicate")), "the log says how many: " + string.Join(" | ", DiagLog.Lines)); + // 5d. One meaning per gesture (issue #80, third report). The property + // that has to hold is that no sequence of presses can make a second + // tile for something already on the screen. + string gestureDir = Path.Combine(dir, "gestures"); + Directory.CreateDirectory(gestureDir); + Store.Init(gestureDir); + + Check(Store.Keep("https://instagram.com/reels", "instagram.com/reels"), + "Keep an address... keeps"); + Check(!Store.Keep("https://instagram.com/reels", "instagram.com/reels") && + Store.AllFavourites.Count == 1, + "keeping the same address again says so and adds nothing"); + Check(!Store.Keep("https://www.instagram.com/reels/", "again") && + Store.AllFavourites.Count == 1, + "nor does keeping another spelling of it"); + + // His report: point at the tile of an address kept from the menu, press + // 8, and it duplicated. Removing is now all that press can do. + Check(Store.RemoveFavourite("https://instagram.com/reels") && + Store.AllFavourites.Count == 0, + "8 on that tile removes it"); + Check(!Store.RemoveFavourite("https://instagram.com/reels"), + "and says so when there is nothing to remove"); + + // The same page is very often in both grids. Each press must act on the + // list the tile was in and leave the other alone. + Store.RecordVisit("https://www.instagram.com/reels", "Instagram"); + Store.Keep("https://www.instagram.com/reels", "instagram.com/reels"); + Check(Store.AllFavourites.Count == 1 && Store.RecentHistory.Count == 1, + "a page can be a favourite and a recent visit at once"); + Check(Store.ForgetVisit("https://www.instagram.com/reels") && + Store.RecentHistory.Count == 0 && Store.AllFavourites.Count == 1, + "8 on the recent tile forgets the visit and keeps the favourite"); + Check(Store.RemoveFavourite("https://www.instagram.com/reels") && + Store.AllFavourites.Count == 0, + "and 8 on the favourite tile removes the favourite"); + + // 8 on the page itself is still a toggle — you are looking at the thing — + // and it must find a favourite kept under any spelling of the address. + Store.Keep("https://instagram.com/reels", "instagram.com/reels"); + Check(!Store.ToggleFavourite("https://www.instagram.com/reels/", "Instagram") && + Store.AllFavourites.Count == 0, + "8 on the page removes what Keep an address... put there"); + // 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