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

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: Jasmine + Karma + Headless Chrome.
Works with vscode debugging for fast test driven development.

Unrelated changes required for compilation:
- options.ts my LitToast interface needed to extend `Element` when
  compiling with karma and tsc. Not sure why it worked with `ts-loader`
- I needed to update webpack.config to accept a partial argv since of
  course we are only using `mode`.

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:
@@ -22,3 +22,4 @@ jobs:
       - run: npm ci
       - run: npm run lint
       - run: npm run build
+      - run: npm test

.gitignore

@@ -1,2 +1,3 @@
 node_modules
 dist
+.vscode

extension/options.ts

@@ -393,7 +393,7 @@ customElements.define('options-form', OptionsForm);
 
 // TODO(https://github.com/Victor-Bernabe/lit-toast/issues/2):
 // Remove this if the component supports types directly.
-interface LitToast {
+interface LitToast extends Element {
   show(text: string, duration: number): Promise<void>;
 }
 declare global {

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;
   }
 
+  #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,32 @@
+type DeepPartial<T> = {
+  [P in keyof T]?: DeepPartial<T[P]>;
+};
+
+const chromeSpy: DeepPartial<typeof chrome> & { reset: Function } = {
+  // Creates the partial structure of chrome web APIs rikaikun tests need.
+  runtime: {
+    sendMessage: () => {},
+    onMessage: {
+      addListener: () => {},
+    },
+  },
+
+  /**
+   * Set's named spies for each method defined above and allows state to be
+   * easily reset between test methods.
+   */
+  reset() {
+    chromeSpy.runtime!.sendMessage = jasmine.createSpy(
+      'chrome.runtime.sendMessage'
+    );
+    chromeSpy.runtime!.onMessage!.addListener = jasmine.createSpy(
+      'chrome.runtime.onMessage.addListener'
+    );
+  },
+};
+
+chromeSpy.reset();
+window.chrome = chromeSpy as unknown as typeof chrome;
+const hardenedChrome = chromeSpy as Required<typeof chromeSpy>;
+
+export { hardenedChrome as chrome };

extension/test/rikaicontent_test.ts

@@ -0,0 +1,64 @@
+// import 'jasmine-sinon';
+// import * as sinonChrome from 'sinon-chrome';
+import { TestOnlyRcxContent } from '../rikaicontent';
+import { chrome } from './chrome_stubs';
+
+// declare const chrome: Required<typeof chromeSpy>;
+
+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).toHaveBeenCalledWith(
+        jasmine.objectContaining({ type: 'xsearch', text: '試す' }),
+        jasmine.any(Function)
+      );
+    });
+  });
+
+  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).toHaveBeenCalledWith(
+        jasmine.objectContaining({ type: 'xsearch', text: '試す' }),
+        jasmine.any(Function)
+      );
+    });
+  });
+});
+
+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
+  );
+}

karma.conf.ts

@@ -0,0 +1,53 @@
+import { Config, ConfigOptions } from 'karma';
+import { Configuration } from 'webpack';
+import webpackConfig from './webpack.config';
+
+// Allows me to use type safe configuration with webpack plugin options.
+interface PluggedConfig extends Omit<Config, 'set'> {
+  set: (config: ConfigOptions & { webpack: Configuration }) => void;
+}
+
+// Load production webpack config for sharing values with test config.
+const webpackDev = webpackConfig({}, { mode: 'development' });
+module.exports = (config: PluggedConfig) => {
+  config.set({
+    frameworks: ['jasmine', 'webpack'],
+    // Load chrome.* API spies early for top level calls.
+    files: ['extension/test/chrome_stubs.ts', 'extension/test/**/*.ts'],
+    preprocessors: {
+      // Typescript files need to be compiled by webpack and then sourcemapped.
+      '**/*.ts': ['webpack', 'sourcemap'],
+    },
+    reporters: ['spec'], // Prints status based on Jasmine specs.
+    port: 9876, // karma web server port
+    colors: true,
+    logLevel: config.LOG_INFO,
+    browsers: ['ChromeHeadlessDebug'],
+    customLaunchers: {
+      ChromeHeadlessDebug: {
+        base: 'ChromeHeadless',
+        flags: ['--remote-debugging-port=9333'],
+      },
+    },
+    // By default, use autowatch to re-run tests on changes.
+    autoWatch: true,
+    concurrency: Infinity,
+    webpack: {
+      module: webpackDev.module,
+      mode: 'development',
+      resolve: webpackDev.resolve,
+      devtool: 'inline-source-map',
+      output: {
+        module: true,
+        // Outputs absolute file paths instead of webpack:///path/to/file.extension
+        // This makes stacktrace paths clickable in a shell (e.g. VS Code)
+        devtoolModuleFilenameTemplate: '[absolute-resource-path]',
+      },
+      experiments: webpackDev.experiments,
+      // Needed for https://github.com/ryanclark/karma-webpack/issues/493
+      optimization: {
+        splitChunks: false,
+      },
+    },
+  });
+};