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

Chrome fixes #1388

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@sarvaje
Member

sarvaje commented Oct 9, 2018

Pull request checklist

Make sure you:

For non-trivial changes, please make sure you also:

  • Added/Updated related documentation.
  • Added/Updated related tests.

Short description of the change(s)

@sarvaje sarvaje requested review from alrra and molant as code owners Oct 9, 2018

@alrra alrra force-pushed the webhintio:master branch from 00e7c0c to 40fb6ab Oct 10, 2018

@molant molant requested a review from antross Oct 10, 2018

@molant

This comment has been minimized.

Member

molant commented Oct 10, 2018

@sarvaje can you please fix the conflicts?

@@ -52,6 +52,9 @@ The set of settings supported by Chrome connector are:
session in headless mode or with GPU disabled. Here's the full list
of [available command line flags][cli flags].
`['--no-default-browser-check']` is the default value.
* `waitForContentLoaded (number)`: Time the browser has to wait for the
event `loadingFinished` before use the body received in the event

This comment has been minimized.

@molant

molant Oct 10, 2018

Member

Time in milliseconds to wait for the loadingFinished event from the debugging protocol before requesting the body of a response. The default value is 10000 (10 seconds).

command line API flags. Useful if you would like to start your
session in headless mode or with GPU disabled. Here's the full list
of [available command line flags][cli flags].
* `waitForContentLoaded (number)`: Time the browser has to wait for the

This comment has been minimized.

@molant

molant Oct 10, 2018

Member

Use the same text as before and maybe put a note that this is only for browsers using the debugging protocol?

This comment has been minimized.

@sarvaje

sarvaje Oct 10, 2018

Member

This is already in a section for remote debugging protocol.

* parts components are lower case even if the original component
* has a different case. [src$="texttofind" i] will do the search
* case insensitive.
* decodeURIComponent is added because the connector return the

This comment has been minimized.

@molant

molant Oct 10, 2018

Member

the connector returns the...

@@ -318,7 +330,29 @@ export class Connector implements IConnector {
}
return new Promise((resolve, reject) => {
this._waitingForLoadingFinished.set(requestId, { reject, resolve });
/*
* There some cases where the a request never dispatch

This comment has been minimized.

@molant

molant Oct 10, 2018

Member

Sometimes the debugging protocol doesn't dispatch the event loadFinished for a request even thought it's finished. To avoid waiting forever, timeout at waitForContentLoaded and continue the process.

sarvaje added some commits Oct 9, 2018

Fix: Add timeout waiting for a request to finish.
Fix: Do not validate url generating selector.

@sarvaje sarvaje force-pushed the sarvaje:chrome-fixes branch from cdb0bba to 39d50cc Oct 10, 2018

@sarvaje

This comment has been minimized.

Member

sarvaje commented Oct 10, 2018

@molant done!

@molant

molant approved these changes Oct 10, 2018

@alrra alrra closed this in f300d04 Oct 10, 2018

@sarvaje sarvaje deleted the sarvaje:chrome-fixes branch Oct 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment