-
Notifications
You must be signed in to change notification settings - Fork 65
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: capture PerformanceNavigationTiming events even when window.load fires before plugin loads #81
Conversation
if ( | ||
window.performance && | ||
window.performance.getEntriesByType(NAVIGATION).length | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this guarantee that window.load
has fired?
Or should we look for a specific event that signals the page has finished loading -- i.e., PerformanceTiming.loadEventEnd
or PerformanceNavigationTiming.loadEventEnd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed offline; The following line to the if statement uses PerformanceNavigationTiming.loadEventEnd
entryTypes: [NAVIGATION] | ||
}); | ||
this.performanceNavigationEventHandlerTimingLevel2( | ||
performance.getEntriesByType(NAVIGATION)[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it guaranteed that there is only one navigation timing entry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, I will change it to iterate instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, just found this: https://www.w3.org/TR/navigation-timing-2/
"Only the current document resource is included in the performance timeline; there is only one PerformanceNavigationTiming object in the performance timeline."
Please modify the commit message to use the following conventions:
|
… fires before plugin loads
b0be4cc
to
3900a8c
Compare
Modified NavigationPlugin to capture PerformanceNavigationEvents when the plugin loads after the page does.
To test the change, do the following:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.