Skip to content

Commit

Permalink
fix: Skip text processing when range is null (#598)
Browse files Browse the repository at this point in the history
Avoid raising errors when range is null. Calling caretRangeFromPoint when "x or y are negative, outside viewport, or there is no text entry node" will return null. One common example is hovering over a scrollbar.

Fixes #386
  • Loading branch information
joshua-webdev authored Jul 17, 2021
1 parent 992bdcc commit ae55bff
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 36 deletions.
9 changes: 8 additions & 1 deletion extension/rikaicontent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1094,7 +1094,14 @@ class RcxContent {
fake.scrollLeft = eventTarget.scrollLeft;
}
// Calculate range and friends here after we've made our fake textarea/input divs.
range = document.caretRangeFromPoint(ev.clientX, ev.clientY);
range = document.caretRangeFromPoint(
ev.clientX,
ev.clientY
) as Range | null;
// If we don't have a valid range, don't do any more work
if (range === null) {
return;
}
const startNode = range.startContainer;
ro = range.startOffset;

Expand Down
89 changes: 54 additions & 35 deletions extension/test/rikaicontent_test.ts
Original file line number Diff line number Diff line change
@@ -1,61 +1,80 @@
import { Config } from '../configuration';
import { TestOnlyRcxContent } from '../rikaicontent';
import { expect, use } from '@esm-bundle/chai';
import chrome from 'sinon-chrome';
import simulant from 'simulant';
import sinon from 'sinon';
import sinonChai from 'sinon-chai';

use(sinonChai);

describe('RcxContent.show', () => {
let rcxContent = new TestOnlyRcxContent();

describe('RcxContent', () => {
beforeEach(() => {
chrome.reset();
rcxContent = new TestOnlyRcxContent();
// Default enable rcxContent since no tests care about that now.
rcxContent.enableTab({ showOnKey: '' } as Config);
});
describe('.show', () => {
describe('when given Japanese word interrupted with text wrapped by `display: none`', () => {
it('sends "xsearch" message with invisible text omitted', () => {
const span = insertHtmlIntoDomAndReturnFirstTextNode(
'<span>試<span style="display:none">test</span>す</span>'
);

executeShowForGivenNode(rcxContent, span);

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>'
);
expect(chrome.runtime.sendMessage).to.have.been.calledWith(
sinon.match({ type: 'xsearch', text: '試す' }),
sinon.match.any
);
});
});

executeShowForGivenNode(rcxContent, span);
describe('when given Japanese word interrupted with text wrapped by `visibility: hidden`', () => {
it('sends "xsearch" message with invisible text omitted', () => {
const span = insertHtmlIntoDomAndReturnFirstTextNode(
'<span>試<span style="visibility: hidden">test</span>す</span>'
);

expect(chrome.runtime.sendMessage).to.have.been.calledWith(
sinon.match({ type: 'xsearch', text: '試す' }),
sinon.match.any
);
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>'
);
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);
executeShowForGivenNode(rcxContent, span);

expect(chrome.runtime.sendMessage).to.have.been.calledWith(
sinon.match({ type: 'xsearch', text: '試す' }),
sinon.match.any
);
expect(chrome.runtime.sendMessage).to.have.been.calledWith(
sinon.match({ type: 'xsearch', text: '試testす' }),
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>'
);
describe('mousemove', () => {
it('handled without logging errors if `caretRangeFromPoint` returns null', () => {
sinon
.stub(document, 'caretRangeFromPoint')
.returns(null as unknown as Range);
sinon.spy(console, 'log');

executeShowForGivenNode(rcxContent, span);
simulant.fire(document, 'mousemove');

expect(chrome.runtime.sendMessage).to.have.been.calledWith(
sinon.match({ type: 'xsearch', text: '試testす' }),
sinon.match.any
);
expect(console.log).to.not.have.been.called;
});
});
});
Expand Down
12 changes: 12 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
"@types/chrome": "0.0.147",
"@types/mocha": "^8.2.2",
"@types/node": "^16.3.3",
"@types/simulant": "^0.2.0",
"@types/sinon-chai": "^3.2.5",
"@types/sinon-chrome": "^2.2.10",
"@web/test-runner": "^0.13.13",
Expand All @@ -78,6 +79,7 @@
"prettier-plugin-jsdoc": "^0.3.23",
"semantic-release": "^17.4.4",
"semantic-release-chrome": "^1.1.3",
"simulant": "^0.2.2",
"sinon": "^7.5.0",
"sinon-chai": "^3.7.0",
"sinon-chrome": "^3.0.1",
Expand Down

0 comments on commit ae55bff

Please sign in to comment.