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