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

Main resource computed artifact does not match finalUrl #4454

Closed
patrickhulce opened this issue Feb 7, 2018 · 0 comments
Closed

Main resource computed artifact does not match finalUrl #4454

patrickhulce opened this issue Feb 7, 2018 · 0 comments
Assignees
Labels

Comments

@patrickhulce
Copy link
Collaborator

In our main-resource computed artifact we use different logic from how our final URL is computed. Significantly, it considers 304 to be redirect. We should unify this logic so issues like #4440 don't happen.

Final URL Update Logic - works pretty well, let's align on this for now

// Update startingUrl if it's ever redirected.
this._monitoredUrl = startingUrl;
this._networkStatusMonitor.on('requestloaded', redirectRequest => {
// Ignore if this is not a redirected request.
if (!redirectRequest.redirectSource) {
return;
}
const earlierRequest = redirectRequest.redirectSource;
if (earlierRequest.url === this._monitoredUrl) {
this._monitoredUrl = redirectRequest.url;
}
});

Main Resource Logic - fails in cases like #4440

/**
* @param {WebInspector.NetworkRequest} record
* @return {boolean}
*/
isMainResource(request) {
return request.statusCode < HTTP_REDIRECT_CODE_LOW ||
request.statusCode > HTTP_REDIRECT_CODE_HIGH;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants