Skip to content

Commit

Permalink
feat(detection): Ignore invisible nodes when extracting text under mouse
Browse files Browse the repository at this point in the history
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
  • Loading branch information
melink14 committed Jul 16, 2021
1 parent 1bdd3d3 commit 73d825a
Show file tree
Hide file tree
Showing 9 changed files with 2,141 additions and 134 deletions.
9 changes: 7 additions & 2 deletions .github/workflows/node.js.yml
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -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:
Expand All @@ -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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
node_modules
dist
.vscode
coverage
2 changes: 1 addition & 1 deletion .prettierrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module.exports = {
arrowParens: 'always',
overrides: [
{
files: ['*.js', '*.ts'],
files: ['*.js', '*.mjs', '*.ts'],
options: {
parser: 'jsdoc-parser',
},
Expand Down
13 changes: 13 additions & 0 deletions extension/rikaicontent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand All @@ -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
Expand Down Expand Up @@ -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 };
2 changes: 2 additions & 0 deletions extension/test/chrome_stubs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import * as sinonChrome from 'sinon-chrome';
window.chrome = sinonChrome as unknown as typeof chrome;
81 changes: 81 additions & 0 deletions extension/test/rikaicontent_test.ts
Original file line number Diff line number Diff line change
@@ -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
);
}
Loading

0 comments on commit 73d825a

Please sign in to comment.