Skip to content
Permalink
Browse files

Fix: Tie more elements from DOM snapshot back to original source

Also fix some cases where use of the snapshot vs. original document were
inconsistent between connectors (except connector-local which needs to
be different).
  • Loading branch information...
antross committed Mar 11, 2019
1 parent 5859e56 commit 29b22740ad4948d424a2072349dc8dfb7bc99134
Showing with 373 additions and 46 deletions.
  1. +4 βˆ’1 packages/connector-chrome/tests/collect.ts
  2. +3 βˆ’0 packages/connector-chrome/tests/evaluate.ts
  3. +4 βˆ’1 packages/connector-chrome/tests/events.ts
  4. +4 βˆ’1 packages/connector-chrome/tests/fetch-content.ts
  5. +4 βˆ’1 packages/connector-chrome/tests/invalid-url.ts
  6. +4 βˆ’1 packages/connector-chrome/tests/request-response.ts
  7. +1 βˆ’0 packages/connector-jsdom/package.json
  8. +11 βˆ’2 packages/connector-jsdom/src/connector.ts
  9. +4 βˆ’1 packages/connector-jsdom/tests/collect.ts
  10. +3 βˆ’0 packages/connector-jsdom/tests/evaluate.ts
  11. +4 βˆ’1 packages/connector-jsdom/tests/events.ts
  12. +4 βˆ’1 packages/connector-jsdom/tests/fetch-content.ts
  13. +4 βˆ’1 packages/connector-jsdom/tests/invalid-url.ts
  14. +4 βˆ’1 packages/connector-jsdom/tests/request-response.ts
  15. +1 βˆ’0 packages/connector-jsdom/tsconfig.json
  16. +4 βˆ’8 packages/connector-local/src/connector.ts
  17. +1 βˆ’0 packages/extension-browser/package.json
  18. +34 βˆ’10 packages/extension-browser/src/content-script/connector.ts
  19. +3 βˆ’1 packages/extension-browser/src/content-script/webhint.ts
  20. +1 βˆ’0 packages/extension-browser/tsconfig.json
  21. +1 βˆ’1 packages/hint/src/lib/hint-context.ts
  22. +28 βˆ’8 packages/hint/src/lib/types/html.ts
  23. +3 βˆ’3 packages/hint/src/lib/utils/dom/create-html-document.ts
  24. +73 βˆ’0 packages/hint/src/lib/utils/dom/find-original-element.ts
  25. +2 βˆ’2 packages/hint/tests/lib/types/html.ts
  26. +154 βˆ’0 packages/hint/tests/lib/utils/dom/find-original-element.ts
  27. +1 βˆ’0 packages/utils-debugging-protocol-common/package.json
  28. +8 βˆ’1 packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts
  29. +1 βˆ’0 packages/utils-debugging-protocol-common/tsconfig.json
@@ -28,7 +28,10 @@ test.beforeEach((t) => {
emit(): boolean {
return false;
},
async emitAsync(): Promise<any> { }
async emitAsync(): Promise<any> { },
on(): Engine {
return null as any;
}
} as any;

t.context.engineEmitSpy = sinon.spy(engine, 'emit');
@@ -53,6 +53,9 @@ test(`[${name}] Evaluate JavaScript`, async (t) => {
return false;
},
async emitAsync(): Promise<any> { },
on(): Engine {
return null as any;
},
timeout: 10000
} as any;

@@ -206,7 +206,10 @@ test.beforeEach((t) => {
emit(): boolean {
return false;
},
async emitAsync(): Promise<any> { }
async emitAsync(): Promise<any> { },
on(): Engine {
return null as any;
}
} as any;

t.context.engineEmitAsyncSpy = sinon.spy(engine, 'emitAsync');
@@ -18,7 +18,10 @@ test(`[${name}] Fetch Content`, async (t) => {
emit(): boolean {
return false;
},
async emitAsync(): Promise<any> { }
async emitAsync(): Promise<any> { },
on(): Engine {
return null as any;
}
} as any;

const file = fs.readFileSync(path.join(__dirname, './fixtures/common/nellie.png'));
@@ -13,7 +13,10 @@ test(`[${name}] Load an invalid url throws an error`, async (t) => {
emit(): boolean {
return false;
},
async emitAsync(): Promise<any> { }
async emitAsync(): Promise<any> { },
on(): Engine {
return null as any;
}
} as any;

const connector: IConnector = new ChromeConnector(engine, {});
@@ -39,7 +39,10 @@ test(`[${name}] requestResponse`, async (t) => {
emit(): boolean {
return false;
},
async emitAsync(): Promise<any> { }
async emitAsync(): Promise<any> { },
on(): Engine {
return null as any;
}
} as any;

const engineEmitAsyncSpy = sinon.spy(engine, 'emitAsync');
@@ -15,6 +15,7 @@
},
"description": "hint connector for JSDOM",
"devDependencies": {
"@hint/parser-html": "^2.0.2",
"@hint/utils-create-server": "^3.0.0",
"@types/jsdom": "^12.2.3",
"@types/lodash": "^4.14.123",
@@ -45,6 +45,7 @@ import { Engine } from 'hint/dist/src/lib/engine';
import isHTMLDocument from 'hint/dist/src/lib/utils/network/is-html-document';
import createHTMLDocument from 'hint/dist/src/lib/utils/dom/create-html-document';
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';
@@ -67,6 +68,7 @@ export default class JSDOMConnector implements IConnector {
private _targetNetworkData!: NetworkData;
private _window!: Window;
private _document!: HTMLDocument;
private _originalDocument: HTMLDocument | undefined;
private _timeout: number;
private _resourceLoader?: ResourceLoader;
private _subprocesses: Set<ChildProcess>;
@@ -82,6 +84,12 @@ export default class JSDOMConnector implements IConnector {
this.server = server;
this._timeout = server.timeout;
this._subprocesses = new Set();

(server as Engine<import('@hint/parser-html').HTMLEvents>).on('parse::end::html', (event) => {
if (!this._originalDocument) {
this._originalDocument = event.document;
}
});
}

/*
@@ -228,7 +236,7 @@ export default class JSDOMConnector implements IConnector {

const jsdom = new JSDOM(this._targetNetworkData.response.body.content, {
beforeParse: beforeParse(this.finalHref),
includeNodeLocations: true,
// includeNodeLocations: true, // TODO: re-enable once locations can be copied from snapshot.
pretendToBeVisual: true,
resources: this._resourceLoader,
runScripts: 'dangerously',
@@ -250,9 +258,10 @@ export default class JSDOMConnector implements IConnector {

debug(`${this.finalHref} loaded, traversing`);
try {
// TODO: Use a DOM snapshot and copy node locations insead of serializing.
const html = this._window.document.documentElement.outerHTML;

const htmlDocument = createHTMLDocument(html);
const htmlDocument = createHTMLDocument(html, this._originalDocument);

this._document = htmlDocument;

@@ -29,7 +29,10 @@ test.beforeEach((t) => {
emit(): boolean {
return false;
},
async emitAsync(): Promise<any> { }
async emitAsync(): Promise<any> { },
on(): Engine {
return null as any;
}
} as any;

t.context.engineEmitSpy = sinon.spy(engine, 'emit');
@@ -53,6 +53,9 @@ test(`[${name}] Evaluate JavaScript`, async (t) => {
return false;
},
async emitAsync(): Promise<any> { },
on(): Engine {
return null as any;
},
timeout: 10000
} as any;
const connector: IConnector = new JSDOMConnector(engine, {});
@@ -197,7 +197,10 @@ test(`[${name}] Events`, async (t) => {
emit(): boolean {
return false;
},
async emitAsync(): Promise<any> { }
async emitAsync(): Promise<any> { },
on(): Engine {
return null as any;
}
} as any;

const engineEmitAsyncSpy = sinon.spy(engine, 'emitAsync');
@@ -19,7 +19,10 @@ test(`[${name}] Fetch Content`, async (t) => {
emit(): boolean {
return false;
},
async emitAsync(): Promise<any> { }
async emitAsync(): Promise<any> { },
on(): Engine {
return null as any;
}
} as any;

const connector: IConnector = new JSDOMConnector(engine, {});
@@ -13,7 +13,10 @@ test(`[${name}] Load an invalid url throws an error`, async (t) => {
emit(): boolean {
return false;
},
async emitAsync(): Promise<any> { }
async emitAsync(): Promise<any> { },
on(): Engine {
return null as any;
}
} as any;

const connector: IConnector = new JSDOMConnector(engine, {});
@@ -39,7 +39,10 @@ test(`[${name}] requestResponse`, async (t) => {
emit(): boolean {
return false;
},
async emitAsync(): Promise<any> { }
async emitAsync(): Promise<any> { },
on(): Engine {
return null as any;
}
} as any;

const engineEmitAsyncSpy = sinon.spy(engine, 'emitAsync');
@@ -14,6 +14,7 @@
],
"references": [
{ "path": "../hint" },
{ "path": "../parser-html" },
{ "path": "../utils-connector-tools" },
{ "path": "../utils-create-server" }
]
@@ -267,7 +267,7 @@ export default class LocalConnector implements IConnector {
return new JSDOM(html, {

/** Needed to provide line/column positions for elements. */
includeNodeLocations: true,
// includeNodeLocations: true, // TODO: re-enable once locations can be copied from snapshot.

/**
* Needed to let hints run script against the DOM.
@@ -280,14 +280,10 @@ export default class LocalConnector implements IConnector {

/* istanbul ignore next */
private async onParseHTML(event: HTMLParse) {
const document = event.document;
const jsdom: JSDOM = this.createJsdom(document.pageHTML());
this._document = event.document;
this._evaluate = this.createJsdom(event.html).window.eval;

this._evaluate = jsdom.window.eval;

this._document = document;

await traverse(document, this.engine, event.resource);
await traverse(this._document, this.engine, event.resource);

await this.engine.emitAsync('can-evaluate::script', { resource: this._href } as CanEvaluateScript);
}
@@ -31,6 +31,7 @@
"@hint/hint-validate-set-cookie-header": "^2.1.2",
"@hint/hint-x-content-type-options": "^3.0.1",
"@hint/parser-css": "^2.1.0",
"@hint/parser-html": "^2.0.2",
"@hint/parser-javascript": "^2.1.0",
"@hint/parser-manifest": "^2.0.2",
"@hint/utils-create-server": "^3.0.0",
@@ -20,8 +20,10 @@ import createHTMLDocument from 'hint/dist/src/lib/utils/dom/create-html-document
import traverse from 'hint/dist/src/lib/utils/dom/traverse';

export default class WebExtensionConnector implements IConnector {
private _document = createHTMLDocument(document.documentElement.outerHTML);
private _document: HTMLDocument | undefined;
private _originalDocument: HTMLDocument | undefined;
private _engine: Engine;
private _fetchEndQueue: FetchEnd[] = [];
private _onComplete: (resource: string) => void = () => { };
private _options: ConnectorOptionsConfig;

@@ -33,6 +35,12 @@ export default class WebExtensionConnector implements IConnector {
this._options.waitFor = 1000;
}

(engine as Engine<import ('@hint/parser-html').HTMLEvents>).on('parse::end::html', (event) => {
if (event.resource === location.href) {
this._originalDocument = event.document;
}
});

browser.runtime.onMessage.addListener(async (events: Events) => {
if (events.fetchEnd) {
await this.notifyFetch(events.fetchEnd);
@@ -51,11 +59,13 @@ export default class WebExtensionConnector implements IConnector {
setTimeout(async () => {

if (document.documentElement) {
this._document = createHTMLDocument(document.documentElement.outerHTML);
this._document = createHTMLDocument(document.documentElement.outerHTML, this._originalDocument);

await traverse(this._document, this._engine, resource);
}

await this.sendFetchEndEvents();

this._onComplete(resource);
}, this._options.waitFor);
};
@@ -71,10 +81,18 @@ export default class WebExtensionConnector implements IConnector {
browser.runtime.sendMessage(message);
}

private async sendFetchEndEvents() {
for (const event of this._fetchEndQueue) {
await this.notifyFetch(event);
}
}

private setFetchElement(event: FetchEnd) {
const url = event.request.url;

event.element = getElementByUrl(this._document, url);
if (this._document) {
event.element = getElementByUrl(this._document, url);
}
}

private setFetchType(event: FetchEnd): string {
@@ -87,13 +105,19 @@ export default class WebExtensionConnector implements IConnector {
}

private async notifyFetch(event: FetchEnd) {
/*
* Delay dispatching FetchEnd until we have the DOM snapshot to populate `element`.
* But immediately process target's FetchEnd to populate `originalDocument`.
*/
if (!this._document && event.response.url !== location.href) {
this._fetchEndQueue.push(event);

return;
}

this.setFetchElement(event);
const type = this.setFetchType(event);

if (event.response.url === location.href) {
this._document = createHTMLDocument(event.response.body.content);
}

await this._engine.emitAsync(`fetch::end::${type}` as 'fetch::end::*', event);
}

@@ -154,20 +178,20 @@ export default class WebExtensionConnector implements IConnector {
}

public querySelectorAll(selector: string): HTMLElement[] {
return this._document.querySelectorAll(selector);
return this._document ? this._document.querySelectorAll(selector) : [];
}

/* istanbul ignore next */
public close() {
return Promise.resolve();
}

public get dom(): HTMLDocument {
public get dom(): HTMLDocument | undefined {
return this._document;
}

/* istanbul ignore next */
public get html(): string {
return this._document.pageHTML();
return this._document ? this._document.pageHTML() : '';
}
}
@@ -8,6 +8,7 @@ import { Configuration } from 'hint/dist/src/lib/config';
import { HintResources, HintsConfigObject, IHintConstructor } from 'hint/dist/src/lib/types';

import CSSParser from '@hint/parser-css';
import HTMLParser from '@hint/parser-html';
import JavaScriptParser from '@hint/parser-javascript';
import ManifestParser from '@hint/parser-manifest';

@@ -71,7 +72,7 @@ const main = async (userConfig: Config) => {
hints: hintsConfig,
hintsTimeout: 10000,
ignoredUrls: determineIgnoredUrls(userConfig.ignoredUrls),
parsers: ['css', 'javascript', 'manifest']
parsers: ['css', 'html', 'javascript', 'manifest']
};

const resources: HintResources = {
@@ -82,6 +83,7 @@ const main = async (userConfig: Config) => {
missing: [],
parsers: [
CSSParser as any,
HTMLParser as any,
JavaScriptParser as any,
ManifestParser as any
]
@@ -37,6 +37,7 @@
{ "path": "../hint-validate-set-cookie-header" },
{ "path": "../hint-x-content-type-options" },
{ "path": "../parser-css" },
{ "path": "../parser-html" },
{ "path": "../parser-javascript" },
{ "path": "../parser-manifest" },
{ "path": "../utils-create-server" }
@@ -102,7 +102,7 @@ export class HintContext<E extends Events = Events> {
}

/** Finds the approximative location in the page's HTML for a match in an element. */
public findProblemLocation(element: HTMLElement): ProblemLocation {
public findProblemLocation(element: HTMLElement): ProblemLocation | null {
return element.getLocation();
}

Oops, something went wrong.

0 comments on commit 29b2274

Please sign in to comment.
You can’t perform that action at this time.