1963464 - Implement `PaintTimingMixin` without the `presentationTime` as a first step

Status: RESOLVED FIXED

(Core :: DOM: Performance APIs, enhancement)

Description

[Nazım Can Altınova [:canova]]

This is the definition of PaintTimingMixin:

[Exposed=Window]
interface mixin PaintTimingMixin {
    readonly attribute DOMHighResTimeStamp paintTime;
    readonly attribute DOMHighResTimeStamp? presentationTime;
};

presentationTime returns an implementation-defined time. It is nullable and browsers might choose to not implement it. We would like to implement it eventually but it requires a lot of plumbing. So as a first step, we would like to implement the mixin without this attribute. And then as a second step we will implement it in 1950098.

Comment 1

[Nazım Can Altınova [:canova]]

Attachment: Bug 1963464 - Initial implementation of PaintTimingMixin for the PaintTiming and LCP entries r?sefeng

This patch implements PaintTimingMixin with only the paintTime, without the
presentationTime. The presentationTime property is nullable in the spec and
the browsers might choose to not implement it. We would like to implement it
in the future, but we will do that as a follow-up.

Comment 2

[Nazım Can Altınova [:canova]]

Attachment: Bug 1963464 - Update the wpt test checks and the test expectations r?sefeng

We had to update the presentationTime checks because they were only checking
if it was undefined. But spec mentions presentationTime as nullable. So
"presentationTime" in entry will return true for browsers that implement the
PaintTimingMixin but choose to not implement the presentationTime property.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2c1297577960
Initial implementation of PaintTimingMixin for the PaintTiming and LCP entries r=sefeng,webidl
https://hg.mozilla.org/integration/autoland/rev/7272221822b0
Update the wpt test checks and the test expectations r=sefeng

Comment 4

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/52325 for changes under testing/web-platform/tests

Comment 5

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/2c1297577960
https://hg.mozilla.org/mozilla-central/rev/7272221822b0

Comment 6

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 7

[Sebastian Zartner [:sebo]]

The implementation of paintTime should be documented at https://developer.mozilla.org/en-US/docs/Web/API/PerformancePaintTiming and https://developer.mozilla.org/en-US/docs/Web/API/LargestContentfulPaint and their related browser compat data plus the release notes for Firefox 140.

One question, though: Nazım Can, the definition of the LargestContentfulPaint interface currently doesn't include the PaintTimingMixin. What's the reason for that?

Sebastian

Comment 8

[Nazım Can Altınova [:canova]]

Good question. The LCP spec was updated in this PR to include the PaintTimingMixin. And in the source code I can see that line. But it appears that the rendered version is not updated...

I also see that Sean created an issue one month ago mentioning that the rendered version of the spec is not updated and still points to a version from Jan 2024: https://github.com/w3c/largest-contentful-paint/issues/130

So I believe it should be updated if we can trigger a build. I'll try to chase it.

Comment 9

[Nazım Can Altınova [:canova]]

There was a build error. Let's see if this will fix it: https://github.com/w3c/largest-contentful-paint/pull/132

Comment 10

[Nazım Can Altınova [:canova]]

Looks like it's fixed after that PR. I can see the PaintTimingMixin in the idl definition now: https://w3c.github.io/largest-contentful-paint/#sec-largest-contentful-paint-interface

Comment 11

[Sebastian Zartner [:sebo]]

Perfect! Thank you for the quick fix!

Sebastian

Comment 12

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

The corresponding spec PR https://github.com/w3c/largest-contentful-paint/pull/126 removed renderTime from LargestContentfulPaint but we still have it, should there be a tracking bug?

Comment 13

[Nazım Can Altınova [:canova]]

Good question, Kagami. I'm not so sure if that was intentional. I will reach out to the spec authors to double check this. But looking at the idl definition on Chrome, they still support renderTime, so even if that was intentional, I wouldn't remove it just now so we don't create an interop issue.

I filed bug 1966759 to track this.

Comment 14

[Nazım Can Altınova [:canova]]

Ah, looks like you were faster than me: https://github.com/w3c/largest-contentful-paint/issues/133