feat(detection): Ignore invisible nodes when extracting text under mo…

…use (#561)

Specifically, ignore `display: none` and `visibility: hidden`.

This change is also the first unit tested change so contains the initial
configuration of testing. (See #159 for details on decisions).

Testing Stack: @web/test-runner + Mocha/Chai/Sinon + Puppeteer Headless Chrome.
Works with vscode debugging for fast test driven development.

With --coverage flag, we get a coverage report that we can upload to
codecov.

Fixes #366
Fixes #159

.github/workflows/node.js.yml

@@ -1,7 +1,7 @@
 # This workflow will do a clean install of node dependencies, build the source code and run tests across different versions of node
 # For more information see: https://help.github.com/actions/language-and-framework-guides/using-nodejs-with-github-actions
 
-name: Lint/Build
+name: Lint/Build/Test
 
 on:
   pull_request:
@@ -10,7 +10,7 @@ on:
 
 jobs:
   build_and_lint:
-    name: Build and lint rikaikun
+    name: Build, lint and test rikaikun
     runs-on: ubuntu-latest
 
     steps:
@@ -23,3 +23,8 @@ jobs:
       - run: npm run lint
       - run: npm run checktypes
       - run: npm run build
+      - run: npm test -- --coverage
+      - name: Coverage
+        env:
+          CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
+        run: npm i -g codecov && codecov --disable=gcov

.gitignore

@@ -1,3 +1,4 @@
 node_modules
 dist
 .vscode
+coverage

.prettierrc.js

@@ -5,7 +5,7 @@ module.exports = {
   arrowParens: 'always',
   overrides: [
     {
-      files: ['*.js', '*.ts'],
+      files: ['*.js', '*.mjs', '*.ts'],
       options: {
         parser: 'jsdoc-parser',
       },

extension/rikaicontent.ts

@@ -671,6 +671,12 @@ class RcxContent {
       text.length < maxLength &&
       (nextNode = result.iterateNext() as Text | null)
     ) {
+      // Since these nodes are text nodes we can assume parentElement exists.
+      if (!this.isElementVisible(nextNode.parentElement!)) {
+        // If a node isn't visible it shouldn't be part of our text look up.
+        // See issue #366 for an example where it breaks look ups.
+        continue;
+      }
       endIndex = Math.min(nextNode.data.length, maxLength - text.length);
       text += nextNode.data.substring(0, endIndex);
       selEndList.push({ node: nextNode, offset: endIndex });
@@ -679,6 +685,11 @@ class RcxContent {
     return text;
   }
 
+  private isElementVisible(element: HTMLElement) {
+    const style = window.getComputedStyle(element);
+    return style.visibility !== 'hidden' && style.display !== 'none';
+  }
+
   // given a node which must not be null,
   // returns either the next sibling or next sibling of the father or
   // next sibling of the fathers father and so on or null
@@ -1286,3 +1297,5 @@ chrome.runtime.onMessage.addListener((request) => {
 
 // When a page first loads, checks to see if it should enable script
 chrome.runtime.sendMessage({ type: 'enable?' });
+
+export { RcxContent as TestOnlyRcxContent };

extension/test/chrome_stubs.ts

@@ -0,0 +1,2 @@
+import * as sinonChrome from 'sinon-chrome';
+window.chrome = sinonChrome as unknown as typeof chrome;

extension/test/rikaicontent_test.ts

@@ -0,0 +1,81 @@
+import { TestOnlyRcxContent } from '../rikaicontent';
+import { expect, use } from '@esm-bundle/chai';
+import chrome from 'sinon-chrome';
+import sinon from 'sinon';
+import sinonChai from 'sinon-chai';
+
+use(sinonChai);
+
+describe('RcxContent.show', () => {
+  beforeEach(() => {
+    chrome.reset();
+  });
+
+  describe('when given Japanese word interrupted with text wrapped by `display: none`', () => {
+    it('sends "xsearch" message with invisible text omitted', () => {
+      const rcxContent = new TestOnlyRcxContent();
+      const span = insertHtmlIntoDomAndReturnFirstTextNode(
+        '<span>試<span style="display:none">test</span>す</span>'
+      );
+
+      executeShowForGivenNode(rcxContent, span);
+
+      expect(chrome.runtime.sendMessage).to.have.been.calledWith(
+        sinon.match({ type: 'xsearch', text: '試す' }),
+        sinon.match.any
+      );
+    });
+  });
+
+  describe('when given Japanese word interrupted with text wrapped by `visibility: hidden`', () => {
+    it('sends "xsearch" message with invisible text omitted', () => {
+      const rcxContent = new TestOnlyRcxContent();
+      const span = insertHtmlIntoDomAndReturnFirstTextNode(
+        '<span>試<span style="visibility: hidden">test</span>す</span>'
+      );
+
+      executeShowForGivenNode(rcxContent, span);
+
+      expect(chrome.runtime.sendMessage).to.have.been.calledWith(
+        sinon.match({ type: 'xsearch', text: '試す' }),
+        sinon.match.any
+      );
+    });
+  });
+
+  describe('when given Japanese word is interrupted with text wrapped by visible span', () => {
+    it('sends "xsearch" message with all text included', () => {
+      const rcxContent = new TestOnlyRcxContent();
+      const span = insertHtmlIntoDomAndReturnFirstTextNode(
+        '<span>試<span>test</span>す</span>'
+      );
+
+      executeShowForGivenNode(rcxContent, span);
+
+      expect(chrome.runtime.sendMessage).to.have.been.calledWith(
+        sinon.match({ type: 'xsearch', text: '試testす' }),
+        sinon.match.any
+      );
+    });
+  });
+});
+
+function insertHtmlIntoDomAndReturnFirstTextNode(htmlString: string): Node {
+  const template = document.createElement('template');
+  template.innerHTML = htmlString;
+  return document.body.appendChild(template.content.firstChild!);
+}
+
+function executeShowForGivenNode(
+  rcxContent: TestOnlyRcxContent,
+  node: Node
+): void {
+  rcxContent.show(
+    {
+      prevRangeNode: rcxContent.getFirstTextChild(node) as Text,
+      prevRangeOfs: 0,
+      uofs: 0,
+    },
+    0
+  );
+}