Remove a favourite by pointing at its tile

docs/INTERNALS.md

@@ -1639,7 +1639,42 @@ 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…*.
+rid of one.
+
+### A favourite you can add and cannot remove
+
+That message — "open a page first, or use *Keep an address…*" — was the correct
+thing to say and useless advice, and his next report said so within the hour: two
+tiles both reading `instagram.com`, both kept by typing, neither removable by any
+spelling he could see.
+
+The hole is worth naming plainly, because it had been there since favourites
+existed and only #80 made it reachable: **removal required knowing the string.**
+Being on the page and pressing `8` worked because the engine handed us the exact
+address; anything else meant typing it back. And what the start screen shows is a
+*host* with the `www.` stripped and a *title* — neither of which is what the
+favourite is stored as. Two kept addresses on one site therefore render as the
+same two lines, and the thing you would type is nowhere on the screen. Adding was
+one press; removing was a guess.
+
+Three changes, and the first is the one that matters:
+
+- **`8` on the start screen removes the tile the pointer is on.** `PageScript.linkAt()`
+  climbs from the hit-test to the nearest anchor and returns its `href`, which for
+  a tile is the stored address exactly, because `HomePage` wrote it there. Pointing
+  at the thing you want gone is the one gesture that cannot be spelled wrong, and
+  the pointer already knew how to hit-test — the whole addition is fifteen lines of
+  script and a bridge message. A *recent* tile gets kept instead, which is the same
+  key doing what it does everywhere else.
+- **A kept address is named by its address.** It was named by `SiteRules.KeyFor`,
+  i.e. the bare host, which is why his two tiles read identically. `Urls.Readable`
+  drops the scheme and a trailing slash and keeps the rest.
+- **`SameKey` folds a leading `www.` as well as the trailing slash** — the two
+  parts of an address a person neither sees nor types. Without it, typing back
+  exactly what the tile shows still missed the favourite that tile names.
+
+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.
 
 ## Settings that belong to a site, not to the browser
 

src/common/PageScript.cs

@@ -204,6 +204,35 @@ function scrollableAncestor(node) {
 
     hide: function () { if (st.el) { st.el.style.display = 'none'; } },
 
+    /* The address of the link under the pointer, or ''. The start screen's tiles
+       are plain links (see HomePage), and this is how the app acts on the tile
+       somebody is pointing at without following it.
+
+       Issue #80 is why it exists. A favourite kept by typing its address could be
+       added and never taken away: removing one meant typing the same address
+       again, and the tile does not show an address — it shows a host and a title,
+       neither of which is what the favourite is stored as. So the reporter had two
+       tiles that read identically, pointed at different pages, and could not be
+       removed by any spelling he could see. Pointing at the thing you want gone is
+       the gesture that cannot go wrong, and the pointer already knows how to
+       hit-test. */
+    linkAt: function () {
+      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
+             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'));
+        }
+
+        n = n.parentElement || n.parentNode;
+        if (n && n.nodeType !== 1) { return ''; }
+      }
+
+      return '';
+    },
+
     /* fx, fy are fractions of the viewport, so the native side never needs to
        know the page's CSS pixel size or zoom level. */
     move: function (fx, fy) {

src/common/Store.cs

@@ -313,10 +313,15 @@ public static void Set(string key, bool value)
         /// 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.
+        /// The trailing slash on the path is folded, and a leading <c>www.</c> on
+        /// the host — the two parts of an address a person neither sees nor types.
+        /// The start screen shows a tile's host with the <c>www.</c> already
+        /// stripped, so without that half, typing back exactly what is on the
+        /// screen still failed to find the favourite it names.
+        ///
+        /// 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.
         /// </summary>
         private static string SameKey(string url)
         {
@@ -349,6 +354,15 @@ private static string SameKey(string url)
                 head = head.Substring(0, head.Length - 1);
             }
 
+            // And the www. the tile does not show. Only at the start of the host,
+            // never anywhere else in the address.
+            if (authority >= 0 &&
+                head.Length > floor + 4 &&
+                string.Compare(head, floor, "www.", 0, 4, StringComparison.OrdinalIgnoreCase) == 0)
+            {
+                head = head.Substring(0, floor) + head.Substring(floor + 4);
+            }
+
             return head + url.Substring(cut);
         }
 

src/common/Urls.cs

@@ -30,6 +30,27 @@ public static string Normalize(string input)
                 : "https://duckduckgo.com/?q=" + Uri.EscapeDataString(trimmed);
         }
 
+        /// <summary>
+        /// An address as somebody would read it out: no scheme, no trailing slash.
+        ///
+        /// Used to name a favourite that was kept by typing its address rather
+        /// than by being on it, because there is no page title to take and the
+        /// host on its own is not a name — issue #80's reporter had two tiles both
+        /// reading "instagram.com", pointing at different pages, with nothing on
+        /// the screen to tell them apart or to type back in.
+        /// </summary>
+        public static string Readable(string url)
+        {
+            string rest = url ?? string.Empty;
+            int scheme = rest.IndexOf("://", StringComparison.Ordinal);
+            if (scheme > 0)
+            {
+                rest = rest.Substring(scheme + 3);
+            }
+
+            return rest.Length > 1 ? rest.TrimEnd('/') : rest;
+        }
+
         /// <summary>
         /// Quotes a string for embedding in injected JavaScript. Typed text reaches
         /// the page through a script, so an unescaped quote would break the script

src/elm/BrowserApp.cs

@@ -793,7 +793,7 @@ private void DrawHints()
                 new[] { "5", "type in the field you clicked" },
                 new[] { "6", "fit page — when the page is cut off" },
                 new[] { "7", "hide this card" },
-                new[] { "8", "keep this page as a tile" },
+                new[] { "8", "keep this page — on the start screen, remove a tile" },
                 new[] { "9", "start screen" },
                 new[] { "Info", "images off — kept for this site" },
             };
@@ -1274,6 +1274,10 @@ private void OnBridgeMessage(JavaScriptMessage message)
                     DiagLog.Add("viewport fix: " + (parts.Length > 1 ? parts[1] : "?"));
                     break;
 
+                case "tile":
+                    ToggleTile(parts.Length > 1 ? parts[1] : null);
+                    break;
+
                 case "typed":
                     DiagLog.Add("typed: " + (parts.Length > 1 ? parts[1] : "?"));
                     break;
@@ -1805,6 +1809,23 @@ private string PageUrl()
         /// </summary>
         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.
+                try
+                {
+                    _web.Eval("try{window." + BridgeName + ".postMessage('tile\u0001'+String(window." +
+                              PageScript.Namespace + ".linkAt()));}catch(e){}");
+                }
+                catch (Exception ex)
+                {
+                    DiagLog.Add("asking for the tile failed: " + ex.Message);
+                }
+
+                return;
+            }
+
             string url = PageUrl();
             if (string.IsNullOrEmpty(url) || url == "-")
             {
@@ -1821,6 +1842,56 @@ private void ToggleFavourite()
             _cachedUrl = url;
         }
 
+        /// <summary>
+        /// Keeps or removes the favourite whose tile the pointer is on.
+        ///
+        /// 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.
+        ///
+        /// A recent tile gets kept rather than removed, which is the same key doing
+        /// the same thing it does everywhere else.
+        /// </summary>
+        private void ToggleTile(string url)
+        {
+            if (string.IsNullOrEmpty(url) || url == "null" || Store.IsGenerated(url))
+            {
+                Flash("Point at a tile first, then press 8");
+                return;
+            }
+
+            bool kept = Store.ToggleFavourite(url, TitleForTile(url));
+            DiagLog.Add((kept ? "kept tile " : "removed tile ") + url);
+            Flash((kept ? "Kept " : "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();
+        }
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        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);
+        }
+
         /// <summary>The page's title now, not as of the last status refresh.</summary>
         private string PageTitle()
         {
@@ -1855,7 +1926,10 @@ private void KeepAddress(string 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.
-            string title = SiteRules.KeyFor(url) ?? url;
+            // 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");

src/nui/NuiBrowserApp.cs

@@ -1210,7 +1210,7 @@ private void BuildOverlay()
             new[] { "5", "video: TV overlay / in page — if black" },
             new[] { "6", "fit page — when the page is cut off" },
             new[] { "7", "hide this card" },
-            new[] { "8", "keep this page as a tile" },
+            new[] { "8", "keep this page — on the start screen, remove a tile" },
             new[] { "9", "start screen" },
             new[] { "Info", "images off — kept for this site" },
         };
@@ -2357,6 +2357,25 @@ private string PageUrl()
         /// </summary>
         private void ToggleFavourite()
         {
+            if (_atHome)
+            {
+                // On the start screen the answer is a tile, and the page has to be
+                // asked which one.
+                try
+                {
+                    _web.EvaluateJavaScript(
+                        "String(window." + PageScript.Namespace + " && window." +
+                        PageScript.Namespace + ".linkAt())",
+                        result => ToggleTile(result));
+                }
+                catch (Exception ex)
+                {
+                    DiagLog.Add("asking for the tile failed: " + ex.Message);
+                }
+
+                return;
+            }
+
             string url = PageUrl();
             if (string.IsNullOrEmpty(url) || url == "-")
             {
@@ -2373,6 +2392,56 @@ private void ToggleFavourite()
             _cachedUrl = url;
         }
 
+        /// <summary>
+        /// Keeps or removes the favourite whose tile the pointer is on.
+        ///
+        /// 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.
+        ///
+        /// A recent tile gets kept rather than removed, which is the same key doing
+        /// the same thing it does everywhere else.
+        /// </summary>
+        private void ToggleTile(string url)
+        {
+            if (string.IsNullOrEmpty(url) || url == "null" || Store.IsGenerated(url))
+            {
+                Flash("Point at a tile first, then press 8");
+                return;
+            }
+
+            bool kept = Store.ToggleFavourite(url, TitleForTile(url));
+            DiagLog.Add((kept ? "kept tile " : "removed tile ") + url);
+            Flash((kept ? "Kept " : "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();
+        }
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        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);
+        }
+
         /// <summary>The page's title now, not as of the last status refresh.</summary>
         private string PageTitle()
         {
@@ -2397,9 +2466,10 @@ 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.
-            string title = SiteRules.KeyFor(url) ?? url;
+            // 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");

tools/startpage/Program.cs

@@ -184,6 +184,21 @@ private static int Main(string[] args)
             Store.ToggleFavourite("https://one.test/", "one");
             Check(!Store.IsFavourite("https://two.test/"), "two sites are never one favourite");
 
+            // www. is the other half a person never types — the start screen shows
+            // a tile's host with it already stripped, so typing back exactly what
+            // is on the screen has to find the favourite it names.
+            Store.ToggleFavourite("https://www.instagram.com/reels", "instagram.com/reels");
+            Check(Store.IsFavourite("https://instagram.com/reels"),
+                  "the host as the tile shows it is the same favourite");
+            Check(Store.IsFavourite("https://instagram.com/reels/"),
+                  "with or without the slash as well");
+            Check(!Store.IsFavourite("https://wwwinstagram.com/reels"),
+                  "but only a whole www. label at the front of the host");
+            Check(!Store.IsFavourite("https://mail.www.test/reels"),
+                  "and never a www. anywhere else in it");
+            Check(!Store.IsFavourite("https://www.instagram.com/reel"),
+                  "and /reel is still not /reels — the paths are what differ");
+
             // 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");