Merge pull request #240 from chocolateboy/pagination-tweaks

Pagination tweaks and fixes

2 files changed, 30 insertions, 25 deletions

diff --git a/src/content/navigates.js b/src/content/navigates.js
index 3e12a6f..c9baa30 100644
--- a/src/content/navigates.js
+++ b/src/content/navigates.js
@@ -1,18 +1,18 @@
-const PREV_LINK_PATTERNS = [
-  /\bprev\b/i, /\bprevious\b/i, /\bback\b/i,
-  /</, /\u2039/, /\u2190/, /\xab/, /\u226a/, /<</
-];
-
-const NEXT_LINK_PATTERNS = [
-  /\bnext\b/i,
-  />/, /\u203a/, /\u2192/, /\xbb/, /\u226b/, />>/
-];
-
-const findLinkByPatterns = (win, patterns) => {
-  const links = win.document.getElementsByTagName('a');
-  return Array.prototype.find.call(links, (link) => {
-    return patterns.some(ptn => ptn.test(link.textContent));
-  });
+const REL_PATTERN = {
+  prev: /^(?:prev(?:ious)?|older)\b|\u2039|\u2190|\xab|\u226a|<</i,
+  next: /^(?:next|newer)\b|\u203a|\u2192|\xbb|\u226b|>>/i,
+};
+
+// Return the last element in the document matching the supplied selector
+// and the optional filter, or null if there are no matches.
+const selectLast = (win, selector, filter) => {
+  let nodes = win.document.querySelectorAll(selector);
+
+  if (filter) {
+    nodes = Array.from(nodes).filter(filter);
+  }
+
+  return nodes.length ? nodes[nodes.length - 1] : null;
 };
 
 const historyPrev = (win) => {
@@ -23,16 +23,21 @@ const historyNext = (win) => {
   win.history.forward();
 };
 
-const linkCommon = (win, rel, patterns) => {
-  let link = win.document.querySelector(`link[rel~=${rel}][href]`);
+// Code common to linkPrev and linkNext which navigates to the specified page.
+const linkRel = (win, rel) => {
+  let link = selectLast(win, `link[rel~=${rel}][href]`);
 
   if (link) {
-    win.location = link.getAttribute('href');
+    win.location = link.href;
     return;
   }
 
-  link = win.document.querySelector(`a[rel~=${rel}]`) ||
-    findLinkByPatterns(win, patterns);
+  const pattern = REL_PATTERN[rel];
+
+  link = selectLast(win, `a[rel~=${rel}][href]`) ||
+    // `innerText` is much slower than `textContent`, but produces much better
+    // (i.e. less unexpected) results
+    selectLast(win, 'a[href]', lnk => pattern.test(lnk.innerText));
 
   if (link) {
     link.click();
@@ -40,11 +45,11 @@ const linkCommon = (win, rel, patterns) => {
 };
 
 const linkPrev = (win) => {
-  linkCommon(win, 'prev', PREV_LINK_PATTERNS);
+  linkRel(win, 'prev');
 };
 
 const linkNext = (win) => {
-  linkCommon(win, 'next', NEXT_LINK_PATTERNS);
+  linkRel(win, 'next');
 };
 
 const parent = (win) => {
diff --git a/test/content/navigates.test.js b/test/content/navigates.test.js
index d8a3316..f1f0741 100644
--- a/test/content/navigates.test.js
+++ b/test/content/navigates.test.js
@@ -53,7 +53,7 @@ describe('navigates module', () => {
     ));
 
     it('navigates to <a> elements whose text matches "previous"', testPrev(
-      '<a href="#dummy">preview</a><a href="#prev">go to previous</a>'
+      '<a href="#dummy">previously</a><a href="#prev">previous page</a>'
     ));
 
     it('navigates to <a> elements whose decoded text matches "<<"', testPrev(
@@ -119,11 +119,11 @@ describe('navigates module', () => {
     ));
 
     it('prefers link[rel~=next] to a[rel~=next]', testNext(
-      '<a rel="next" href="#dummy">click me<><link rel="next" href="#next" />'
+      '<a rel="next" href="#dummy">click me</a><link rel="next" href="#next" />'
     ));
 
     it('prefers a[rel~=next] to a::text(pattern)', testNext(
-      '<a href="#dummy">go to next</a><a rel="next" href="#next">click me</a>'
+      '<a href="#dummy">next page</a><a rel="next" href="#next">click me</a>'
     ));
   });