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