Prevent opening up intra-page links in separate tab/window in diff view

closes edgi-govdata-archiving#494
SYU15 · May 10, 2020 · ae1626f · ae1626f
1 parent 47645de
commit ae1626f
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 3 deletions.
diff --git a/src/scripts/__tests__/html-transforms.test.js b/src/scripts/__tests__/html-transforms.test.js
@@ -61,7 +61,7 @@ describe('HtmlTransforms module', () => {
     expect(document.querySelector('.wm-diff-other')).toBeInstanceOf(Element);
   });
 
-  test('addTargetBlank adds a target attribute of "_blank" to only <a> tags', () => {
+  test('addTargetBlank adds a target attribute of "_blank" to only <a> tags, excluding intra-page links', () => {
     let document = parser.parseFromString(`<!doctype html>
       <html>
         <head>
@@ -77,6 +77,8 @@ describe('HtmlTransforms module', () => {
             body { background: purple; }
           </style>
           <a href='google.com' id='goo'>Goo test</a>
+          <a href='#main' id='intra1'>Inta test for #</a>
+          <a href='javascript:;' id='intra2'>Intra test for javascript</a>
           <h1 style='color: orange;' id='orange'>Hello</h1>
         </body>
       </html>
@@ -85,5 +87,7 @@ describe('HtmlTransforms module', () => {
     document = addTargetBlank(document);
     expect(document.getElementById('goo').target).toEqual('_blank');
     expect(document.getElementById('orange').target).toBeFalsy();
+    expect(document.getElementById('intra1').target).toBeFalsy();
+    expect(document.getElementById('intra2').target).toBeFalsy();
   });
 });
diff --git a/src/scripts/html-transforms.js b/src/scripts/html-transforms.js
@@ -50,7 +50,7 @@ export function removeStyleAndScript (document) {
 
 /**
  * Prevents navigation from within a diff by forcing links to open in a new
- * tab when clicked. This helps ensure we don’t get in a state where one side
+ * tab when clicked (excluding intra-page links). This helps ensure we don’t get in a state where one side
  * of a side-by-side diff has been navigated and viewer does not realize they
  * are no longer actually looking at a *diff*.
  *
@@ -62,8 +62,14 @@ export function removeStyleAndScript (document) {
 export function addTargetBlank (document) {
   // Add target="_blank" to all <a>tags
   document.querySelectorAll('a').forEach(node => {
-    node.setAttribute('target', '_blank');
+    // set href to empty string in case attribute is null
+    const href = node.getAttribute('href') || '';
+
+    if (href.charAt(0) !== '#' && href.indexOf('javascript:') !== 0) {
+      node.setAttribute('target', '_blank');
+    }
   });
+
   return document;
 }