Clarify iframes in PaintTiming #24
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spanicker
reviewed
Nov 29, 2017
painttiming.bs
Outdated
| @@ -56,9 +56,9 @@ This is formally defined as the when <a href="https://html.spec.whatwg.org/multi | |||
|
|
|||
| NOTE: The rendering pipeline is very complex, and the timestamp should be the latest timestamp the browser is able to note in this pipeline (best effort). Typically the time at which the frame is submitted to the OS for display is recommended for this API. | |||
|
|
|||
| <dfn export>First Paint</dfn> entry contains a DOMHighResTimeStamp reporting the time when the browser first rendered after navigation. This excludes the default background paint, but includes non-default background paint. This is the first key moment developers care about in page load – when the browser has started to render the page. | |||
| <dfn export>First Paint</dfn> entry contains a DOMHighResTimeStamp reporting the time when the browser first rendered after navigation. This excludes the default background paint, but includes non-default background paint and iframes (regardless of the contents of the iframe). This is the first key moment developers care about in page load – when the browser has started to render the page. | |||
There was a problem hiding this comment.
is this really needed? Feels like it raises some questions.
So either remove this OR make it more clear (enclosing box render for iframe)
There was a problem hiding this comment.
Currently, the spec talks about the PaintTiming entries as if the concept was page-specific. However, in reality it is a PerformanceEntry associated to a Window/Worker, so I think that giving the iframe example is useful to keep that in mind?
spanicker
reviewed
Nov 29, 2017
painttiming.bs
Outdated
| @@ -102,11 +102,11 @@ Reporting Paint Timing {#sec-reporting-paint-timing} | |||
| Perform the following steps: | |||
| * If this instance of update the rendering is the first paint, then record the timestamp as |paint-timestamp| and invoke [[#report-paint-timing]] algorithm with two arguments: string "first-paint" and |paint-timestamp|. | |||
|
|
|||
| NOTE: First paint excludes the default background paint, but includes non-default background paint. | |||
| NOTE: First paint excludes the default background paint, but includes non-default background paint and iframes. | |||
There was a problem hiding this comment.
Ditto here. Maybe a more detailed sentence or two about iframes.
spanicker
approved these changes
Nov 29, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@spanicker
@tdresser