-
Notifications
You must be signed in to change notification settings - Fork 30
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
Integration with the HTML spec #141
Conversation
Expose timing values measured in the HTML spec for the different navigation timing values, instead of defining its o own processing model. Depends on whatwg/html#6536 See #136
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.
Thanks for pushing this!! :)
index.html
Outdated
@@ -941,7 +549,7 @@ <h3> | |||
instance, the unloading time reveals how long the previous page takes | |||
to execute its unload handler, which could be used to infer the | |||
user's login status. These attacks have been mitigated by enforcing | |||
the <a>same-origin check</a> algorithm when timing information | |||
the [=same origin=] check algorithm when timing information |
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.
Maybe "these attack are mitigated in HTML (link to algorithm) by enforcing a same origin check..."?
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.
A few more nits and comments
index.html
Outdated
"HTML/parsing.html#the-end:current-document-readiness" title= | ||
'interactive" document readiness'>"interactive"</a> [[HTML]]. | ||
The <dfn>domInteractive</dfn> getter steps are to return | ||
|this|' [=load timing info=]'s [=document load timing info/dom interactive time=].</p> |
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.
Nit: unfortunately, and unlike I previously said, s/|this|'/|this|'s/ :/ (here and elsewhere)
run the following steps: | ||
|
||
<p>A <a>PerformanceNavigationTiming</a> has an associated | ||
[=document load timing info=] <a data-dfn-for="PerformanceResourceTiming"> |
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.
Broken link
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.
I think you need to add HTML to the xref specs: https://github.com/w3c/navigation-timing/pull/141/files#diff-0eb547304658805aad788d320f10bf1f292797b5e6d745a3bf617584da017051L70
index.html
Outdated
|
||
|
||
<p>A <a>PerformanceNavigationTiming</a> has an associated | ||
[=document unload timing info=] <a data-dfn-for="PerformanceNavigationTiming"><dfn>previous document |
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.
Broken link
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.
LGTM
</li> | ||
</ol> | ||
<p> | ||
Some user agents maintain the DOM structure of the document in memory |
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.
Theoretically we would've wanted to keep a similar note in HTML (while making sure the Nav Timing entry is not dispatched nor altered in the BFCache case). Since we're planning to change that anyway (and fire new entries for BFCache), it may not be worth bothering over this, assuming the HTML's side processing model aligns with this statement.
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.
Currently the processing model creates the entry when the new document is created. But I guess if it moves to the BFCache and brought back strange things can occur... Let's move this discussion to the HTML PR.
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.
btw I haven't seen any tests for this, nor did I find it in the Chromium implementation. Maybe this can be omitted or addressed later?
Thanks!! I think you can also add yourself as an editor :) |
Creates "document load timing info" and "document unload timing info" structs which get populated at appropriate points in the navigation and loading algorithms, and eventually handed to the appropriate Navigation Timing algorithms. See w3c/navigation-timing#136 and w3c/navigation-timing#141.
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.
LGTM!!
Expose timing values measured in the HTML spec for the different
navigation timing values, instead of defining its own processing model.
Depends on whatwg/html#6536
See #136
Preview | Diff