From 61f75448a216037251264e496e147299a1e264ae Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 29 Apr 2015 07:44:39 +0100 Subject: Fix text matching/exclusion for text link hints. We need to sort elements by their text length. This allows us to exclude the text of descendants from the hint text generated for their ancestors. Note: Tests not yet fixed. --- content_scripts/link_hints.coffee | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 440e6598..fc95bcd8 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -60,6 +60,12 @@ LinkHints = # For these modes, we filter out those elements which don't have an HREF (since there's nothing we can do # with them). elements = (el for el in elements when el.element.href?) if mode in [ COPY_LINK_URL, OPEN_INCOGNITO ] + if settings.get "filterLinkHints" + # When using text filtering, we sort the elements such that we visit descendants before their ancestors. + # This allows us to exclude the text used for matching descendants from that used for matching their + # ancestors. + textLength = (el) -> el.element.textContent?.length ? 0 + elements = elements.sort (a,b) -> textLength(a) - textLength b hintMarkers = (@createMarkerFor(el) for el in elements) @getMarkerMatcher().fillInMarkers(hintMarkers) @@ -238,8 +244,6 @@ LinkHints = # Remove rects from elements where another clickable element lies above it. nonOverlappingElements = [] # Traverse the DOM from first to last, since later elements show above earlier elements. - # NOTE(smblott). filterHints.generateLinkText also assumes this order when generating the content text for - # each hint. Specifically, we consider descendents before we consider their ancestors. visibleElements = visibleElements.reverse() while visibleElement = visibleElements.pop() rects = [visibleElement.rect] -- cgit v1.2.3 From bc5e9545398e0e74529738e690f16063ae41f8c0 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 29 Apr 2015 08:37:28 +0100 Subject: Fix text matching/exclusion; repair tests. --- content_scripts/link_hints.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'content_scripts') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index fc95bcd8..a5be6c57 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -65,7 +65,7 @@ LinkHints = # This allows us to exclude the text used for matching descendants from that used for matching their # ancestors. textLength = (el) -> el.element.textContent?.length ? 0 - elements = elements.sort (a,b) -> textLength(a) - textLength b + elements.sort (a,b) -> textLength(a) - textLength b hintMarkers = (@createMarkerFor(el) for el in elements) @getMarkerMatcher().fillInMarkers(hintMarkers) -- cgit v1.2.3 From 7917647ccad6712118b08cd86e3b4a6dda763ae3 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 29 Apr 2015 10:41:48 +0100 Subject: Use innerHTML instead of contentText. If we use contentText, then it is possible for a child and its to have exactly the same content text, in which case it is arbitrary as to which is sorted first, hence which we choose (where we should be choosing the child). Using innerHTML instead, we guess that even if the contentText lengths are the same, the innerHTML of the parent will be longer (since it contains that of the child). --- content_scripts/link_hints.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index a5be6c57..3cebac4c 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -64,8 +64,8 @@ LinkHints = # When using text filtering, we sort the elements such that we visit descendants before their ancestors. # This allows us to exclude the text used for matching descendants from that used for matching their # ancestors. - textLength = (el) -> el.element.textContent?.length ? 0 - elements.sort (a,b) -> textLength(a) - textLength b + length = (el) -> el.element.innerHTML?.length ? 0 + elements.sort (a,b) -> length(a) - length b hintMarkers = (@createMarkerFor(el) for el in elements) @getMarkerMatcher().fillInMarkers(hintMarkers) -- cgit v1.2.3