Fix bug relating to duplicate hint strings.

(Not sure when this crept in.)

We need to ensure that we always generate the same hint strings for the
same filter state.  Here, we do this by always using the same mechanism
(@filterLinkHints) to set the hint strings.

2 files changed, 24 insertions, 18 deletions

diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee
index 144400b6..15af15c5 100644
--- a/content_scripts/link_hints.coffee
+++ b/content_scripts/link_hints.coffee
@@ -478,7 +478,7 @@ class FilterHints
         @labelMap[forElement] = labelText
 
   generateHintString: (linkHintNumber) ->
-    numberToHintString linkHintNumber + 1, @linkHintNumbers.toUpperCase()
+    numberToHintString linkHintNumber, @linkHintNumbers.toUpperCase()
 
   generateLinkText: (element) ->
     linkText = ""
@@ -512,8 +512,7 @@ class FilterHints
   fillInMarkers: (hintMarkers) ->
     @generateLabelMap()
     DomUtils.textContent.reset()
-    for marker, idx in hintMarkers
-      marker.hintString = @generateHintString(idx)
+    for marker in hintMarkers
       linkTextObject = @generateLinkText(marker.clickableItem)
       marker.linkText = linkTextObject.text
       marker.showLinkText = linkTextObject.show
@@ -522,7 +521,9 @@ class FilterHints
     @activeHintMarker = hintMarkers[0]
     @activeHintMarker?.classList.add "vimiumActiveHintMarker"
 
-    hintMarkers
+    # We use @filterLinkHints() here (although we know that all of the hints will match) to fill in the hint
+    # strings.  This ensures that we always get hint strings in the same order.
+    @filterLinkHints hintMarkers
 
   getMatchingHints: (hintMarkers, tabCount = 0) ->
     delay = 0
@@ -563,10 +564,7 @@ class FilterHints
 
   # Filter link hints by search string, renumbering the hints as necessary.
   filterLinkHints: (hintMarkers) ->
-    idx = 0
     linkSearchString = @linkTextKeystrokeQueue.join("").trim().toLowerCase()
-    return hintMarkers unless 0 < linkSearchString.length
-
     do (scoreFunction = @scoreLinkHint linkSearchString) ->
       linkMarker.score = scoreFunction linkMarker for linkMarker in hintMarkers
     # The Javascript sort() method is known not to be stable.  Nevertheless, we require (and assume, here)
@@ -575,9 +573,10 @@ class FilterHints
     # filtering here).
     hintMarkers = hintMarkers[..].sort (a,b) -> b.score - a.score
 
+    linkHintNumber = 1
     for linkMarker in hintMarkers
       continue unless 0 < linkMarker.score
-      linkMarker.hintString = @generateHintString idx++
+      linkMarker.hintString = @generateHintString linkHintNumber++
       @renderMarker linkMarker
       linkMarker
 
diff --git a/tests/dom_tests/dom_tests.coffee b/tests/dom_tests/dom_tests.coffee
index dd2f5a5d..a79735ae 100644
--- a/tests/dom_tests/dom_tests.coffee
+++ b/tests/dom_tests/dom_tests.coffee
@@ -212,11 +212,14 @@ context "Filtered link hints",
       @linkHints.deactivateMode()
 
     should "label the images", ->
-      hintMarkers = getHintMarkers()
-      assert.equal "1: alt text", hintMarkers[0].textContent.toLowerCase()
-      assert.equal "2: some title", hintMarkers[1].textContent.toLowerCase()
-      assert.equal "3: alt text", hintMarkers[2].textContent.toLowerCase()
-      assert.equal "4", hintMarkers[3].textContent.toLowerCase()
+      hintMarkers = getHintMarkers().map (marker) -> marker.textContent.toLowerCase()
+      # We don't know the actual hint numbers which will be assigned, so we replace them with "N".
+      hintMarkers = hintMarkers.map (str) -> str.replace /^[1-4]/, "N"
+      assert.equal 4, hintMarkers.length
+      assert.isTrue "N: alt text" in hintMarkers
+      assert.isTrue "N: some title" in hintMarkers
+      assert.isTrue "N: alt text" in hintMarkers
+      assert.isTrue "N" in hintMarkers
 
   context "Input hints",
 
@@ -235,11 +238,15 @@ context "Filtered link hints",
 
     should "label the input elements", ->
       hintMarkers = getHintMarkers()
-      assert.equal "1", hintMarkers[0].textContent.toLowerCase()
-      assert.equal "2", hintMarkers[1].textContent.toLowerCase()
-      assert.equal "3: a label", hintMarkers[2].textContent.toLowerCase()
-      assert.equal "4: a label", hintMarkers[3].textContent.toLowerCase()
-      assert.equal "5", hintMarkers[4].textContent.toLowerCase()
+      hintMarkers = getHintMarkers().map (marker) -> marker.textContent.toLowerCase()
+      # We don't know the actual hint numbers which will be assigned, so we replace them with "N".
+      hintMarkers = hintMarkers.map (str) -> str.replace /^[1-5]/, "N"
+      assert.equal 5, hintMarkers.length
+      assert.isTrue "N" in hintMarkers
+      assert.isTrue "N" in hintMarkers
+      assert.isTrue "N: a label" in hintMarkers
+      assert.isTrue "N: a label" in hintMarkers
+      assert.isTrue "N" in hintMarkers
 
 context "Input focus",