Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Update browser extension to use a copy of the DOM #1922

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 11 additions & 48 deletions packages/connector-jsdom/src/connector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,16 @@ import { getContentTypeData, getType } from 'hint/dist/src/lib/utils/content-typ
import {
HttpHeaders,
IConnector,
ElementFound, Event, FetchEnd, FetchError, TraverseDown, TraverseUp,
Event, FetchEnd, FetchError,
NetworkData
} from 'hint/dist/src/lib/types';
import { JSDOMAsyncHTMLElement, JSDOMAsyncHTMLDocument } from 'hint/dist/src/lib/types/jsdom-async-html';
import { JSDOMAsyncHTMLElement, JSDOMAsyncHTMLDocument, JSDOMAsyncWindow } from 'hint/dist/src/lib/types/jsdom-async-html';
import { Engine } from 'hint/dist/src/lib/engine';
import isHTMLDocument from 'hint/dist/src/lib/utils/network/is-html-document';
import createAsyncWindow from 'hint/dist/src/lib/utils/dom/create-async-window';
import traverse from 'hint/dist/src/lib/utils/dom/traverse';
import { Requester } from '@hint/utils-connector-tools/dist/src/requester';

import CustomResourceLoader from './resource-loader';
import { beforeParse } from './before-parse';

Expand Down Expand Up @@ -109,47 +112,6 @@ export default class JSDOMConnector implements IConnector {
return r.get(uri);
}

/** Traverses the DOM while sending `element::typeofelement` events. */
private async traverseAndNotify(element: HTMLElement) {
const eventName = `element::${element.nodeName.toLowerCase()}` as 'element::*';

debug(`emitting ${eventName}`);
/*
* should we freeze it? what about the other siblings, children,
* parents? We should have an option to not allow modifications
* maybe we create a custom object that only exposes read only
* properties?
*/
const event: ElementFound = {
element: new JSDOMAsyncHTMLElement(element),
resource: this.finalHref
};

await this.server.emitAsync(eventName, event);
for (let i = 0; i < element.children.length; i++) {
const child: HTMLElement = element.children[i] as HTMLElement;

debug('next children');
const traverseDown: TraverseDown = {
element: new JSDOMAsyncHTMLElement(element),
resource: this.finalHref
};

await this.server.emitAsync(`traverse::down`, traverseDown);
await this.traverseAndNotify(child);

}

const traverseUp: TraverseUp = {
element: new JSDOMAsyncHTMLElement(element),
resource: this.finalHref
};

await this.server.emitAsync(`traverse::up`, traverseUp);

return Promise.resolve();
}

/**
* JSDOM doesn't download the favicon automatically, this method:
*
Expand Down Expand Up @@ -200,11 +162,10 @@ export default class JSDOMConnector implements IConnector {

try {
this._targetNetworkData = await this.fetchContent(target);
} catch (err) {
} catch (err) /* istanbul ignore next */ {
const hops: string[] = this.request.getRedirects(err.uri);
const fetchError: FetchError = {
element: null as any,
/* istanbul ignore next */
error: err.error ? err.error : err,
hops,
resource: href
Expand Down Expand Up @@ -293,9 +254,11 @@ export default class JSDOMConnector implements IConnector {

debug(`${this.finalHref} loaded, traversing`);
try {
await this.server.emitAsync('traverse::start', event);
await this.traverseAndNotify(window.document.children[0] as HTMLElement);
await this.server.emitAsync('traverse::end', event);
const html = await this.html;

const asyncWindow = createAsyncWindow(html);

await traverse((asyncWindow as JSDOMAsyncWindow).dom!, this.server, this.finalHref);

// We download only the first favicon found
await this.getFavicon(window.document.querySelector('link[rel~="icon"]'));
Expand Down
5 changes: 5 additions & 0 deletions packages/connector-local/src/connector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import { getAsUri } from 'hint/dist/src/lib/utils/network/as-uri';
import asPathString from 'hint/dist/src/lib/utils/network/as-path-string';
import { getContentTypeData, isTextMediaType, getType } from 'hint/dist/src/lib/utils/content-type';
import { IAsyncHTMLDocument, IAsyncHTMLElement, IAsyncWindow } from 'hint/dist/src/lib/types/async-html';
import { JSDOMAsyncWindow } from 'hint/src/lib/types/jsdom-async-html';
import traverse from 'hint/dist/src/lib/utils/dom/traverse';

import isFile from 'hint/dist/src/lib/utils/fs/is-file';
import cwd from 'hint/dist/src/lib/utils/fs/cwd';
Expand Down Expand Up @@ -258,6 +260,9 @@ export default class LocalConnector implements IConnector {
/* istanbul ignore next */
private async onParseHTML(event: HTMLParse) {
this._window = event.window;

await traverse((event.window as JSDOMAsyncWindow).dom!, this.engine, event.resource);

await this.engine.emitAsync('can-evaluate::script', { resource: this._href } as CanEvaluateScript);
}

Expand Down
7 changes: 4 additions & 3 deletions packages/connector-local/tests/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import delay from 'hint/dist/src/lib/utils/misc/delay';
import asPathString from 'hint/dist/src/lib/utils/network/as-path-string';
import { getAsUri } from 'hint/dist/src/lib/utils/network/as-uri';
import { Engine } from 'hint';
import { Events, FetchEnd, Problem } from 'hint/dist/src/lib/types';
import { FetchEnd, Problem } from 'hint/dist/src/lib/types';
import { HTMLEvents } from '@hint/parser-html';

type SandboxContext = {
sandbox: sinon.SinonSandbox;
Expand All @@ -23,10 +24,10 @@ const mockContext = () => {
clear() { },
async emitAsync(event: Event, data: Problem[]): Promise<any> { },
async notify() { },
on(): Engine<Events> {
on(): Engine<HTMLEvents> {
return null as any;
}
} as Partial<Engine<Events>>;
} as Partial<Engine<HTMLEvents>>;

const chokidar = {
watch(target: string, options: Chokidar.WatchOptions): Stream {
Expand Down
27 changes: 5 additions & 22 deletions packages/extension-browser/src/content-script/connector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import {
FetchEnd,
NetworkData
} from 'hint/dist/src/lib/types';
import { JSDOMAsyncWindow } from 'hint/dist/src/lib/types/jsdom-async-html';
import createAsyncWindow from 'hint/dist/src/lib/utils/dom/create-async-window';
import traverse from 'hint/dist/src/lib/utils/dom/traverse';

import { Events } from '../shared/types';
import { AsyncWindow, AsyncHTMLDocument, AsyncHTMLElement } from './web-async-html';
Expand Down Expand Up @@ -47,12 +50,9 @@ export default class WebExtensionConnector implements IConnector {
await this._engine.emitAsync('can-evaluate::script', { resource });

setTimeout(async () => {
const asyncWindow = createAsyncWindow(document.documentElement.outerHTML);

if (this._window && document.documentElement) {
await this._engine.emitAsync('traverse::start', { resource });
await this.traverseAndNotify(document.documentElement, this._window.document);
await this._engine.emitAsync('traverse::end', { resource });
}
await traverse((asyncWindow as JSDOMAsyncWindow).dom!, this._engine, resource);

this._onComplete(resource);
}, this._options.waitFor);
Expand All @@ -69,23 +69,6 @@ export default class WebExtensionConnector implements IConnector {
browser.runtime.sendMessage(message);
}

/** Traverses the DOM while sending `element::*` events. */
private async traverseAndNotify(node: Element, doc: IAsyncHTMLDocument): Promise<void> {
const element = new AsyncHTMLElement(node, doc);
const name = node.tagName.toLowerCase();
const resource = location.href;

await this._engine.emitAsync(`element::${name}` as 'element::*', { element, resource });
await this._engine.emitAsync(`traverse::down`, { element, resource });

// Recursively traverse child elements.
for (let i = 0; i < node.children.length; i++) {
await this.traverseAndNotify(node.children[i], doc);
}

await this._engine.emitAsync(`traverse::up`, { element, resource });
}

private setFetchElement(event: FetchEnd) {
const url = event.request.url;
const elements = Array.from(document.querySelectorAll('[href],[src]')).filter((element: any) => {
Expand Down
7 changes: 6 additions & 1 deletion packages/extension-browser/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,12 @@ const baseConfig = {
}
]
},
node: { fs: 'empty' },
node: {
child_process: 'empty', // eslint-disable-line camelcase
fs: 'empty',
net: 'mock',
tls: 'mock'
},
output: {
filename: '[name].js',
path: path.resolve(__dirname, 'dist/bundle')
Expand Down
14 changes: 7 additions & 7 deletions packages/hint-compat-api/tests/html-next.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ hintRunner.testHint(hintPath, elementAttrAddedAlwaysTrue, { browserslist: ['> 1%
const elementAttrVersionAddedFalse: HintTest[] = [
{
name: 'Element attributes that have version added as false and not deprecated should fail.',
reports: [{ message: 'srcset attribute of the img element is not supported by ie.', position: { column: 9, line: 5 }}],
reports: [{ message: 'srcset attribute of the img element is not supported by ie.', position: { column: 9, line: 4 } }],
serverConfig: generateHTMLConfig('img-srcset')
}
];
Expand Down Expand Up @@ -87,7 +87,7 @@ hintRunner.testHint(hintPath, elementAddedVersionOfTargetedBrowser, { browsersli
const elementAddedInVersionAfterTargetedBrowserVersion: HintTest[] = [
{
name: 'Elements added in version after targeted browser should fail.',
reports: [{ message: 'video element is not supported by ie 8.', position: { column: 9, line: 5 }}],
reports: [{ message: 'video element is not supported by ie 8.', position: { column: 9, line: 4 } }],
serverConfig: generateHTMLConfig('video')
}
];
Expand Down Expand Up @@ -118,7 +118,7 @@ hintRunner.testHint(hintPath, globalAttrVersionAddedNull, { browserslist: ['last
const globalAttrVersionAddedFalse: HintTest[] = [
{
name: 'Global attributes that have version added as false and not deprecated should fail.',
reports: [{ message: 'global attribute dropzone is not supported by edge, firefox, ie.', position: { column: 9, line: 5 }}],
reports: [{ message: 'global attribute dropzone is not supported by edge, firefox, ie.', position: { column: 9, line: 4 } }],
serverConfig: generateHTMLConfig('global-attr-dropzone')
}
];
Expand Down Expand Up @@ -146,7 +146,7 @@ hintRunner.testHint(hintPath, globalAttrAddedVersionOfTargetedBrowser, { browser
const globalAttrAddedInVersionAfterTargetedBrowserVersion: HintTest[] = [
{
name: 'Global attributes added in version after targeted browser should fail.',
reports: [{ message: 'global attribute class is not supported by firefox 31.', position: { column: 9, line: 5 }}],
reports: [{ message: 'global attribute class is not supported by firefox 31.', position: { column: 9, line: 4 } }],
serverConfig: generateHTMLConfig('div')
}
];
Expand All @@ -169,7 +169,7 @@ hintRunner.testHint(hintPath, inputTypeVersionAddedNull, { browserslist: ['last
const inputTypeVersionAddedFalse: HintTest[] = [
{
name: 'Input types that have version added as false and not deprecated should fail.',
reports: [{ message: 'input type color is not supported by ie.', position: { column: 9, line: 5 }}],
reports: [{ message: 'input type color is not supported by ie.', position: { column: 9, line: 4 } }],
serverConfig: generateHTMLConfig('input-color')
}
];
Expand All @@ -179,7 +179,7 @@ hintRunner.testHint(hintPath, inputTypeVersionAddedFalse, { browserslist: ['ie 9
const inputTypeVersionAddedAfterTargetedBrowsers: HintTest[] = [
{
name: 'Input types added in a version after the targeted browsers should fail.',
reports: [{ message: 'input type color is not supported by chrome 19, firefox 28.', position: { column: 9, line: 5 }}],
reports: [{ message: 'input type color is not supported by chrome 19, firefox 28.', position: { column: 9, line: 4 } }],
serverConfig: generateHTMLConfig('input-color')
}
];
Expand All @@ -193,7 +193,7 @@ hintRunner.testHint(hintPath, inputTypeVersionAddedAfterTargetedBrowsers, { brow
const mixedFeaturedCompatibility: HintTest[] = [
{
name: 'Features with mixed compatibility (not supported for specific version and never supported) and not deprecated should throw errors for browsers in which the feature is not supported.',
reports: [{ message: 'integrity attribute of the link element is not supported by edge, ie, safari, safari_ios, samsunginternet_android 4, webview_android 4.', position: { column: 9, line: 5 }}],
reports: [{ message: 'integrity attribute of the link element is not supported by edge, ie, safari, safari_ios, samsunginternet_android 4, webview_android 4.', position: { column: 9, line: 4 } }],
serverConfig: generateHTMLConfig('link-integrity')
}
];
Expand Down
16 changes: 8 additions & 8 deletions packages/hint-compat-api/tests/html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ hintRunner.testHint(hintPath, removedForFlags, { browserslist: ['firefox 34'] })
const onlySupportedByFlags: HintTest[] = [
{
name: 'Elements only supported by flags should fail.',
reports: [{ message: 'shadow element is not supported by firefox 60.', position: { column: 9, line: 5 }}],
reports: [{ message: 'shadow element is not supported by firefox 60.', position: { column: 9, line: 4 } }],
serverConfig: generateHTMLConfig('shadow')
}
];
Expand Down Expand Up @@ -79,7 +79,7 @@ hintRunner.testHint(hintPath, elementRemovedVersionOfTargetedBrowser, { browsers
const elementRemovedVersionEarlierThanMultipleTargetedBrowser: HintTest[] = [
{
name: 'Elements that were removed in a version before the targeted browser should fail.',
reports: [{ message: 'blink element is not supported by firefox 24-26.'}],
reports: [{ message: 'blink element is not supported by firefox 24-26.' }],
serverConfig: generateHTMLConfig('blink')
}
];
Expand All @@ -99,7 +99,7 @@ hintRunner.testHint(hintPath, elementRemovedVersionEarlierThanTargetedBrowser, {
const elementVersionAddedFalse: HintTest[] = [
{
name: 'Elements that have version added as false should fail.',
reports: [{ message: 'blink element is not supported by chrome.', position: { column: 9, line: 5 }}],
reports: [{ message: 'blink element is not supported by chrome.', position: { column: 9, line: 4 } }],
serverConfig: generateHTMLConfig('blink')
}
];
Expand All @@ -109,7 +109,7 @@ hintRunner.testHint(hintPath, elementVersionAddedFalse, { browserslist: ['last 2
const featureVersionAddedFalseForAllTargetedBrowsers: HintTest[] = [
{
name: 'Features with no support (version added is false) for multiple targeted browsers should fail.',
reports: [{ message: 'element element is not supported by any of your target browsers.', position: { column: 9, line: 5 }}],
reports: [{ message: 'element element is not supported by any of your target browsers.', position: { column: 9, line: 4 } }],
serverConfig: generateHTMLConfig('element')
}
];
Expand All @@ -119,7 +119,7 @@ hintRunner.testHint(hintPath, featureVersionAddedFalseForAllTargetedBrowsers, {
const elementVersionAddedFalseForMultipleBrowsers: HintTest[] = [
{
name: 'Elements that have version added as false for multiple browsers should fail with one error.',
reports: [{ message: 'blink element is not supported by chrome, edge, ie.', position: { column: 9, line: 5 }}],
reports: [{ message: 'blink element is not supported by chrome, edge, ie.', position: { column: 9, line: 4 } }],
serverConfig: generateHTMLConfig('blink')
}
];
Expand All @@ -129,7 +129,7 @@ hintRunner.testHint(hintPath, elementVersionAddedFalseForMultipleBrowsers, { bro
const featureVersionAddedMixedFalseAndNullForDifferentBrowsers: HintTest[] = [
{
name: 'Features with unknown support (version added is null) and no support (version added is false) for different browsers should fail for unsupported browsers.',
reports: [{ message: 'element element is not supported by edge, firefox_android.', position: { column: 9, line: 5 }}],
reports: [{ message: 'element element is not supported by edge, firefox_android.', position: { column: 9, line: 4 } }],
serverConfig: generateHTMLConfig('element')
}
];
Expand All @@ -148,7 +148,7 @@ hintRunner.testHint(hintPath, elementAttrRemovedVersionLaterThanTargetedBrowser,
const elementAttrRemovedVersionOfTargetedBrowser: HintTest[] = [
{
name: 'Element attributes that were removed the version of the targeted browser should fail.',
reports: [{ message: 'scoped attribute of the style element is not supported by firefox 55.'}],
reports: [{ message: 'scoped attribute of the style element is not supported by firefox 55.' }],
serverConfig: generateHTMLConfig('style-scoped')
}
];
Expand All @@ -158,7 +158,7 @@ hintRunner.testHint(hintPath, elementAttrRemovedVersionOfTargetedBrowser, { brow
const elementAttrRemovedVersionEarlierThanTargetedBrowser: HintTest[] = [
{
name: 'Element attributes that were removed in a version before the targeted browser should fail.',
reports: [{ message: 'scoped attribute of the style element is not supported by firefox 56.'}],
reports: [{ message: 'scoped attribute of the style element is not supported by firefox 56.' }],
serverConfig: generateHTMLConfig('style-scoped')
}
];
Expand Down
8 changes: 4 additions & 4 deletions packages/hint-no-protocol-relative-urls/tests/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const tests: HintTest[] = [
name: `'link' with initial // fails the hint`,
reports: [{
message: generateErrorMessage('//site.webmanifest'),
position: { column: 37, line: 2 }
position: { column: 37, line: 1 }
}],
serverConfig: generateHTMLPage('<link rel="manifest" href="//site.webmanifest">')
},
Expand All @@ -46,7 +46,7 @@ const tests: HintTest[] = [
name: `'script' with initial // fails the hint`,
reports: [{
message: generateErrorMessage('//script.js'),
position: { column: 23, line: 5 }
position: { column: 23, line: 4 }
}],
serverConfig: generateHTMLPage(undefined, '<script src="//script.js"></script>')
},
Expand All @@ -66,7 +66,7 @@ const tests: HintTest[] = [
name: `'a' with initial // fails the hint`,
reports: [{
message: generateErrorMessage('//home'),
position: { column: 19, line: 5 }
position: { column: 19, line: 4 }
}],
serverConfig: generateHTMLPage(undefined, '<a href="//home">home</a>')
},
Expand All @@ -76,4 +76,4 @@ const tests: HintTest[] = [
}
];

hintRunner.testHint(getHintPath(__filename), tests);
hintRunner.testHint(getHintPath(__filename), tests, { parsers: ['html'] });
1 change: 1 addition & 0 deletions packages/hint/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"globby": "^9.0.0",
"is-ci": "^2.0.0",
"is-svg": "^3.0.0",
"jsdom": "^13.2.0",
"jsonc-parser": "^2.0.3",
"lodash": "^4.17.11",
"mime-db": "1.35.0",
Expand Down
4 changes: 4 additions & 0 deletions packages/hint/src/lib/types/jsdom-async-html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@ export class JSDOMAsyncWindow implements IAsyncWindow {
return this._document;
}

public get dom(): JSDOM | undefined {
return this._dom;
}

public evaluate(source: string): Promise<any> {
return this._window.eval(source) as any;
}
Expand Down
9 changes: 9 additions & 0 deletions packages/hint/src/lib/utils/dom/create-async-window.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { IAsyncWindow } from '../../types/async-html';
import { JSDOMAsyncWindow } from '../../types/jsdom-async-html';
import createJsdom from './create-jsdom';

export default (html: string, allowScripts: boolean = false): IAsyncWindow => {
const dom = createJsdom(html, allowScripts);

return new JSDOMAsyncWindow(dom.window, dom);
};
Loading