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

Clarify whether First Contentful Paint considers content outside the viewport #58

Closed
smfr opened this issue Mar 3, 2020 · 45 comments · Fixed by #59 or #65
Closed

Clarify whether First Contentful Paint considers content outside the viewport #58

smfr opened this issue Mar 3, 2020 · 45 comments · Fixed by #59 or #65

Comments

@smfr
Copy link

smfr commented Mar 3, 2020

It's unclear now whether elements outside the viewport (generally, lower in the document than the visible portion) are counted for First Contentful Paint criteria.

Can a big video or SVG that's below the fold trigger FCP?

@npm1
Copy link
Contributor

npm1 commented Mar 3, 2020

In Chrome's implementation, we do paint a larger portion of DOM content than what fits within the viewport, and we do not currently do compute intersections with the viewport, so we allow paints outside of the viewport. I think we should allow user agent to consider content outside of the viewport to be FCP, for performance reasons and simplicity. What do you think?

@noamr
Copy link
Contributor

noamr commented Mar 3, 2020

I believe that "when the browser renders pixels to the screen" has to mean "inside the viewport"; there is no screen outside the viewport and not so much point in counting pixels outside the viewport as painted.
This adds some complexity though because in some cases the FCP would be triggered by scrolling (a previously painted element being scrolled into view).

@smfr
Copy link
Author

smfr commented Mar 3, 2020

Similarly, what is the affect of opacity:0 or transform: scale(0.001) on an element being considered for FCP? Do we care about actual pixels rendered, or the existence of boxes?

@noamr
Copy link
Contributor

noamr commented Mar 3, 2020

The spirit of the spec is about pixel rendered, but at a coarse resolution... it's ok if it misses some edge cases as they're unlikely to be valid website use-cases - for example the case where the only content in the viewport is an element with scale(0.001) is unlikely...

I would suggest that scale(0.001) is still considered rendered pixels and opacity: 0 isn't in terms of FCP, but I wonder how chrome handles it.

@anniesullie
Copy link

I made a glitch to test some of the cases:

https://fcp-visibility-edge-cases.glitch.me/

  • Out of viewport currently reports FCP on Chrome
  • opacity: 0 currently does NOT report FCP on Chrome
  • transform: scale(0.001) currently reports FCP on Chrome

I also took a look at Firefox's flag-gated window.performance.timing.timeToNonBlankPaint and it reports the timing in all 3 cases.

@smfr
Copy link
Author

smfr commented Mar 3, 2020

It's probably also worth testing visibility:hidden and things clipped by overflow:hidden.

@anniesullie
Copy link

Thanks, added these test cases. Both visibility:hidden and overflow:hidden do not report FCP on Chrome. All the test cases report timeToNonBlankPaint on Firefox.

@rniwa
Copy link

rniwa commented Mar 3, 2020

The spirit of the spec is about pixel rendered, but at a coarse resolution... it's ok if it misses some edge cases as they're unlikely to be valid website use-cases - for example the case where the only content in the viewport is an element with scale(0.001) is unlikely...

Missing some edge cases is okay but we still need a precise definition in the spec so that we can have multiple independent interoperable implementations.

@othermaciej
Copy link
Member

I think we should allow user agent to consider content outside of the viewport to be FCP, for performance reasons and simplicity. What do you think?

I think part of the question is not just whether it's allowed, but wether it's required. Do paints outside the viewport always count? Only some? Variable per-browser? Saying that this is totally browser-dependent is not good for interop, so that wouldn't be the best option.

@othermaciej
Copy link
Member

From notes above, it seems like Chrome is somewhat random about whether different forms of invisible paints count for FCP. The spec should be explicit about which are supposed to count to make it possible to achieve interop.

@noamr
Copy link
Contributor

noamr commented Mar 4, 2020

I think we can make the spec very clear on one hand and allow optimizations/edge-cases on the other hand.

What matters for web developers who use this API here is the user experience - measuring when the users can actually consume/see content.
Content outside the viewport, invisible text, content with opacity:0, scale(really-small-number), clipped content with overflow/clip-path, canvas that was only painted with the background image or transparent pixels: all are not really "contentful paints".

I don't know how this works in spec-land, but my suggestion is that on one hand the spec would take the strict path regarding what's considered contentful-paint (if it's invisible or just a bg-color it's not contentful), and on the other hand leave room for browsers to optimize and find a fast semi-accurate path to achieve results that are "close enough" to the spirit of first-contentful-paint and would likely do the trick for 99% of websites and make implementations possible.

@othermaciej
Copy link
Member

I think we should assume that, eventually, web developers will come to depend on the edge cases of almost anything. So it's not really ok for a spec to require browsers to be "semi-accurate" or "close enough". If it does that, then it probably hasn't been specified enough to allow interop. I can see how it's convenient for the first implementation to not spell out the details, but it's dangerous for the second and subsequent implementations.

For the offscreen vs onscreen issue, we could say any of the following:

  1. Painting outside the viewport always counts.
  2. Painting outside the viewport counts as long as long as it's within X px of the edge of the viewport.
  3. Painting outside the viewport never counts.
  4. Painting outside the viewport MAY count.
  5. Painting outside the viewport always counts, but browsers are allowed to not paint if they don't expect anything to paint within the viewport.

I think any of these options would be better than not addressing the issue at all.

I suspect that what Chrome has implemented is (5). It might be that what it has implemented is (2) though. I can't tell from the informal descriptions above. I think what we want in the spec is probably (5). And (5) is sort of equivalent to (1), since nothing in this spec requires browsers to paint at any specific time. (Though it's inconvenient if browsers don't paint close to the time when conditions for "first contentful paint are met.) So the "but" clause of (5) could even be a non-normative note. I think either (2) or (5) would be better for interop than (4).

@noamr
Copy link
Contributor

noamr commented Mar 4, 2020

I like (2) better than (5) because it focuses on the user rather than the browser; Also the "viewport" should probably be "the viewport at load time", or to specify how this works with scrolling.

btw none of this requires the browsers to paint, the whole spec is about when contentful paint is measured, not about when it's performed. Does the spec really need to include language about when browsers should/shouldn't paint? Isn't that a place where browsers can differentiate?

@npm1
Copy link
Contributor

npm1 commented Mar 4, 2020

(2) is more precise but too inflexible to work for all user agents precisely because they could use different strategies for preemptive painting outside of the viewport. We currently do (5), it seems good to me to add that to the spec.

npm1 added a commit that referenced this issue Mar 5, 2020
FP and FCP must consider contents that are painted outside of the viewport. This is only relevant in odd edge cases and is done for simplicity and performance. Fixes #58
@npm1 npm1 closed this as completed in #59 Mar 5, 2020
@rniwa
Copy link

rniwa commented Mar 5, 2020

Re-opening the issue since the PR didn't make it clear which viewport is being used.

@rniwa rniwa reopened this Mar 5, 2020
@npm1
Copy link
Contributor

npm1 commented Mar 6, 2020

Sure, I can add a link. It doesn't matter which one is used because what the text means is that all paints count (whether inside or outside the viewport).

npm1 added a commit that referenced this issue Mar 6, 2020
A request was made to add a link to the viewport.
Fixes #58
@rniwa
Copy link

rniwa commented Mar 6, 2020

Ugh... no, I wasn’t talking about the linking. Is it the initial viewport or the actual viewport. The term “viewport” is ambiguous on its own when talking about whether something is in it or not.

@rniwa rniwa reopened this Mar 6, 2020
@rniwa
Copy link

rniwa commented Mar 6, 2020

Can we stop keep landing PRs and closing this issue without fully understanding what the issue is?

@rniwa
Copy link

rniwa commented Mar 6, 2020

The spec also doesn’t define what it means for something to be inside or outside whichever viewport we’re talking about. That too requires a formal definition. For starters, what geometry of what content are we talking about? The bounding box? All these things need to be sorted out before we can land a PR & close this issue.

@yoavweiss
Copy link
Contributor

Apologies. I think I now have a better understanding of your concerns.

At the same time, it seems like that the FP and FCP definitions cover "any rendering (that fits the FP/FCP criteria) regardless of viewport location, in whatever viewport". So it seems to me we could avoid mentioning any kind of viewport in the normative text, and just add the viewport mention as a non-normative clarification.

Would that work?

@rniwa
Copy link

rniwa commented Mar 6, 2020

That might work if the definition of something getting painted does not depend on viewport at all; e.g. involves all or any CSS box generated for the document and doesn’t care its location.

We might need to be mindful of websites that put content outside the viewport for accessibility purposes though... not sure if it’s common to have those contents before anything painted, or if that’s common enough to warrant a special treatment.

@noamr
Copy link
Contributor

noamr commented Mar 8, 2020

Consider <div style='position:absolute; left: -1000px'>Some text here</div>

See PR: #66
Also covers cases of scale(0.001). The idea is that the threshold for an element to become "paintable" is to be intersecting with the document. In both these cases the element would not intersect with the document.

@npm1
Copy link
Contributor

npm1 commented Mar 9, 2020

Ugh... no, I wasn’t talking about the linking. Is it the initial viewport or the actual viewport. The term “viewport” is ambiguous on its own when talking about whether something is in it or not.

Initial viewport and actual viewport are confusing, and there is an open issue to rename them. What I'd link here is 'visual viewport' but I didn't see it defined anywhere. As Yoav and I have noted, 'any paint counts' is the answer to the 'underlying issue'.

Consider <div style='position:absolute; left: -1000px'>Some text here</div>

I don't follow why that is an interesting case. Is that inside a viewport, under any definition of viewport?

I think the best solution would be to remove the invocation of "mark paint timing" from Step 3 of HTML's "update the rendering" steps. It's not clear to me why it is there, because a paint that doesn't happen cannot reasonably be considered a First Paint or First Contentful Paint.

It's there for security reasons. If an attacker adds a single visited styled link whose visibility depends on whether the user has visited the site, then paint timing must report paint values in both cases. I can see how that section is confusing though. Perhaps it would be better to note that under certain conditions the user agent must paint that document, even if the paint operation is not going to produce any changes.

@noamr
Copy link
Contributor

noamr commented Mar 9, 2020

It's there for security reasons. If an attacker adds a single visited styled link whose visibility depends on whether the user has visited the site, then paint timing must report paint values in both cases. I can see how that section is confusing though. Perhaps it would be better to note that under certain conditions the user agent must paint that document, even if the paint operation is not going to produce any changes.

How can visibility depend on visited link status? :visited pseudo-element doesn't allow that.

@smfr
Copy link
Author

smfr commented Mar 9, 2020

Ugh... no, I wasn’t talking about the linking. Is it the initial viewport or the actual viewport. The term “viewport” is ambiguous on its own when talking about whether something is in it or not.

Initial viewport and actual viewport are confusing, and there is an open issue to rename them. What I'd link here is 'visual viewport' but I didn't see it defined anywhere.

The as-yet-unpublished css-viewport spec will define it. Still working on the first ED.

@npm1
Copy link
Contributor

npm1 commented Mar 9, 2020

How can visibility depend on visited link status? :visited pseudo-element doesn't allow that.

If you have a webpage with a single link with a visited style of color: white then the page will look completely white when the link has been visited. Maybe this should be solved from the CSS spec defining that styling?

@noamr
Copy link
Contributor

noamr commented Mar 9, 2020

How can visibility depend on visited link status? :visited pseudo-element doesn't allow that.

If you have a webpage with a single link with a visited style of color: white then the page will look completely white when the link has been visited. Maybe this should be solved from the CSS spec defining that styling?

It will still be considered paintable and contentful from the point of view of FCP.

@npm1
Copy link
Contributor

npm1 commented Mar 9, 2020

Ok, then we can remove the mark the paint timing for documents for which rendering is not necessary.

@rniwa
Copy link

rniwa commented Mar 9, 2020

Ugh... no, I wasn’t talking about the linking. Is it the initial viewport or the actual viewport. The term “viewport” is ambiguous on its own when talking about whether something is in it or not.

Initial viewport and actual viewport are confusing, and there is an open issue to rename them. What I'd link here is 'visual viewport' but I didn't see it defined anywhere. As Yoav and I have noted, 'any paint counts' is the answer to the 'underlying issue'.

Sounds like we're being blocked by the css-viewport spec then.

Consider <div style='position:absolute; left: -1000px'>Some text here</div>

I don't follow why that is an interesting case. Is that inside a viewport, under any definition of viewport?

It's interesting because it's outside the layout or visual viewport, and it would never come into either viewport. This is a common technique used to have text for accessibility purposes but not to render it to the user. e.g. used to basically add alt text for a background image. I don't think we want to consider this text as a part of the FCP if at all possible.

@noamr
Copy link
Contributor

noamr commented Mar 10, 2020

Ugh... no, I wasn’t talking about the linking. Is it the initial viewport or the actual viewport. The term “viewport” is ambiguous on its own when talking about whether something is in it or not.

Initial viewport and actual viewport are confusing, and there is an open issue to rename them. What I'd link here is 'visual viewport' but I didn't see it defined anywhere. As Yoav and I have noted, 'any paint counts' is the answer to the 'underlying issue'.

Sounds like we're being blocked by the css-viewport spec then.

Consider <div style='position:absolute; left: -1000px'>Some text here</div>

I don't follow why that is an interesting case. Is that inside a viewport, under any definition of viewport?

It's interesting because it's outside the layout or visual viewport, and it would never come into either viewport. This is a common technique used to have text for accessibility purposes but not to render it to the user. e.g. used to basically add alt text for a background image. I don't think we want to consider this text as a part of the FCP if at all possible.

I agree, added it to my PR.

@noamr
Copy link
Contributor

noamr commented Mar 10, 2020

Ugh... no, I wasn’t talking about the linking. Is it the initial viewport or the actual viewport. The term “viewport” is ambiguous on its own when talking about whether something is in it or not.

Initial viewport and actual viewport are confusing, and there is an open issue to rename them. What I'd link here is 'visual viewport' but I didn't see it defined anywhere. As Yoav and I have noted, 'any paint counts' is the answer to the 'underlying issue'.

Sounds like we're being blocked by the css-viewport spec then.

I believe that paint-timing spec should not have anything to do with viewports (based on previous discussion here). when "Mark Paint Timing" is called (it's an event loop consideration if/when to call it), "first-contentful-paint" entry should be added if there are paintable & contentful elements in the document. I do agree that the rendering-loop spec needs to be clearer about visual vs. initial viewport.

domenic pushed a commit to whatwg/html that referenced this issue Mar 12, 2020
This addresses the concern surfaced in
w3c/paint-timing#58 (comment).
This call was added to address concerns about visited link issues, but
the agreement on that issue is that this edge case is solved by
requiring user agents to paint when such a link is present even if such
link is styled by color: white (or whichever default background color).
@sefeng211
Copy link

We have a fcp-invisible-text which expects FCP for invisible text (rgba(0, 0, 0, 0)). Based on the above discussion, I'd assume that FCP shouldn't be reported here?

@noamr
Copy link
Contributor

noamr commented Mar 23, 2020

We have a fcp-invisible-text which expects FCP for invisible text (rgba(0, 0, 0, 0)). Based on the above discussion, I'd assume that FCP shouldn't be reported here?

FCP should be reported for that text. Text color does not affect contentfulness.

@mstange
Copy link

mstange commented Mar 23, 2020

(2) is more precise but too inflexible to work for all user agents precisely because they could use different strategies

Interoperability is more important than flexibility for user agents.
In Firefox, invisible text is not added to the display list during paint, so the particular case that's exercised by the fcp-invisible-text test already impacts "flexibility" for user agent implementation.

@noamr
Copy link
Contributor

noamr commented Mar 23, 2020

(2) is more precise but too inflexible to work for all user agents precisely because they could use different strategies

Interoperability is more important than flexibility for user agents.
In Firefox, invisible text is not added to the display list during paint, so the particular case that's exercised by the fcp-invisible-text test already impacts "flexibility" for user agent implementation.

I think it would be better to remain interoperable where possible... I think it's reasonable to make fully-transparent text (rgba/hsla with a==0) non-contentful. It shouldn't have a visited-link privacy issue since RGBA/HSLA is not allowed for :visited. @rniwa, @npm1 ?

@npm1
Copy link
Contributor

npm1 commented Mar 23, 2020

Just to clarify, visibility: hidden makes the text not eligible for FCP at first, and then changing it to visible triggers FCP. Does Firefox not paint text that is marked visible but with the same color as the background? For example, if I change the background to blue (or any non-white) and then add text of that same color, is that text not painted?

@npm1
Copy link
Contributor

npm1 commented Mar 23, 2020

In general I don't think we want to get into the problem of detecting actual visibility of text. I could add a background image that is of some color and then add text of that same color. There seem to be many ways of achieving this kind of 'invisible text'.

@mstange
Copy link

mstange commented Mar 23, 2020

Does Firefox not paint text that is marked visible but with the same color as the background?

Oh, no, detecting that would be way too complicated. Firefox only has a simple "is text transparent" check.

@noamr
Copy link
Contributor

noamr commented Mar 23, 2020

I think that in WebKit implementing this wouldn't be difficult; It would also need to account for text-shadow like you do in nsTextFrame. We don't need to go as far as background detection (also that would start having a :visited link implication).

@npm1
Copy link
Contributor

npm1 commented Mar 23, 2020

Ah I see. That seem reasonable to me too (we'll need to change Chromium implementation though).

@noamr
Copy link
Contributor

noamr commented Mar 24, 2020

Since this issue is about viewport, I created a new one (#75) specifically for transparent text (text with alpha=0)

@noamr
Copy link
Contributor

noamr commented Mar 31, 2020

I believe this can be closed now?

@rniwa
Copy link

rniwa commented Apr 1, 2020

Yup.

@rniwa rniwa closed this as completed Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants