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

The Paint Timing API is now supported by Safari #139

Merged
merged 1 commit into from
May 6, 2021

Conversation

tomayac
Copy link
Member

@tomayac tomayac commented Apr 27, 2021

That is, the stable Safari that ships with iOS 14.5 or macOS 11.4.

That is, the stable Safari that ships with iOS 14.5 or macOS 11.4.
@tunetheweb
Copy link
Member

BTW it seems a little flakey.

I've a demo page here: https://www.tunetheweb.com/experiments/web-vitals/

And here's the results from Safari 14.1 desktop after several reloads:

Safari screenshot

As you can see continually reloading the page sometimes leads to FCP being logged, but often doesn't.

Any ideas why?

Seems to be logged a bit more often in Safari Technology Preview (14.2) but still does miss it quite often.

Reloading the page in Chrome, it looks to be consistently logged.

Not sure if this should be raised as a separate issue, or whether it's a Safari issue, or an issue with the Web-Vitals library but thought I'd ask the question here first in case it influences the decision to merge this change or not.

@tomayac
Copy link
Member Author

tomayac commented Apr 27, 2021

The above comment FYI @noamr, the implementer.

@noamr
Copy link

noamr commented Apr 27, 2021

Not sure what call gives you this console message.... Calling this: performance.getEntriesByType('paint') worked for me after every reload, couldn't reproduce a scenario when it didn't

@tunetheweb
Copy link
Member

It's this library that produces those logs. Agree it does appear to be there consistently (I added a script to the demo page to log the results of performance.getEntriesByType('paint') on load and it's consistently there).

@philipwalton / @tomayac want me to raise a separate issue for this to move the discussion out of this PR?

@tomayac
Copy link
Member Author

tomayac commented Apr 27, 2021

It's this library that produces those logs. Agree it does appear to be there consistently (I added a script to the demo page to log the results of performance.getEntriesByType('paint') on load and it's consistently there).

@philipwalton / @tomayac want me to raise a separate issue for this to move the discussion out of this PR?

I'm yielding to Phil. Just bearer of the news here, not a subject matter expert…

@philipwalton
Copy link
Member

@noamr Can you clarify whether the Safari implementation supports use with PerformanceObserver and the buffered flag? Just looking at @bazzadp's demo, I'm assuming that's why it's not being logged.

@philipwalton
Copy link
Member

I don't have Safari 14.5 yet, but based on running the following in STP, it looks like the buffered flag is not supported:

new PerformanceObserver(console.log).observe({type: 'paint', buffered: true});

@noamr
Copy link

noamr commented Apr 29, 2021

@noamr Can you clarify whether the Safari implementation supports use with PerformanceObserver and the buffered flag? Just looking at @bazzadp's demo, I'm assuming that's why it's not being logged.

AFAIK The buffered flag is not supported at all in WebKit (for any performance entry). PerformanceObserver should work (without buffered). See https://github.com/web-platform-tests/wpt/tree/master/paint-timing/fcp-only for usage of FCP in a compatible way.

@philipwalton
Copy link
Member

philipwalton commented Apr 29, 2021

@noamr the buffered flag is supported for resource entry types (at least in STP). I can verify by running the following code in the console:

new PerformanceObserver((l)=>console.log(l.getEntries())).observe({type:'resource', buffered: true});

Looks like it buffered support may have been implemented after paint timing was implemented though. @npm1 did you end up filing a bug for this that we can link to? If not, I can.

@noamr
Copy link

noamr commented Apr 29, 2021

@noamr the buffered flag is supported for resource entry types (at least in STP). I can verify by running the following code in the console:

new PerformanceObserver((l)=>console.log(l.getEntries())).observe({type:'resource', buffered: true});

Looks like it buffered support may have been implemented after paint timing was implemented though. @npm1 did you end up filing a bug for this that we can link to? If not, I can.

Ah yes, Igalia did this after paint timing went in. Good to know!

@npm1
Copy link

npm1 commented Apr 29, 2021

I haven't filed a bug on WebKit, but does seem like it was an omission. I can create a buffered flag test on fcp-only folder if that helps...

@noamr
Copy link

noamr commented May 2, 2021

I haven't filed a bug on WebKit, but does seem like it was an omission. I can create a buffered flag test on fcp-only folder if that helps...

That and a bug in webkit would probably help...

@npm1
Copy link

npm1 commented May 3, 2021

Filed https://bugs.webkit.org/show_bug.cgi?id=225305

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request May 3, 2021
Relevant GitHub issue:
GoogleChrome/web-vitals#139

Change-Id: I2884dd4c3432920011e984f6a08ebaf6a2176b67
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request May 3, 2021
Relevant GitHub issue:
GoogleChrome/web-vitals#139

Change-Id: I2884dd4c3432920011e984f6a08ebaf6a2176b67
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request May 3, 2021
Relevant GitHub issue:
GoogleChrome/web-vitals#139

Change-Id: I2884dd4c3432920011e984f6a08ebaf6a2176b67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2867865
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#878531}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request May 3, 2021
Relevant GitHub issue:
GoogleChrome/web-vitals#139

Change-Id: I2884dd4c3432920011e984f6a08ebaf6a2176b67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2867865
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#878531}
@philipwalton
Copy link
Member

#145 should address the lack of buffered support until https://bugs.webkit.org/show_bug.cgi?id=225305 is addressed. Once a version with that PR is released, I'll merge this PR.

@philipwalton philipwalton merged commit 5eeb04d into GoogleChrome:master May 6, 2021
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 8, 2021
… test, a=testonly

Automatic update from web-platform-tests
[PaintTiming] Add fcp-only buffered flag test

Relevant GitHub issue:
GoogleChrome/web-vitals#139

Change-Id: I2884dd4c3432920011e984f6a08ebaf6a2176b67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2867865
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#878531}

--

wpt-commits: c2f801a0f01edffed2e271530f23d6dae1f5277e
wpt-pr: 28797
@noamr
Copy link

noamr commented Feb 10, 2022

Patch for the buffered thingy: https://bugs.webkit.org/show_bug.cgi?id=225305
I totally missed this, it's quite an easy fix.

mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
Relevant GitHub issue:
GoogleChrome/web-vitals#139

Change-Id: I2884dd4c3432920011e984f6a08ebaf6a2176b67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2867865
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#878531}
NOKEYCHECK=True
GitOrigin-RevId: fdbdd8b448210f42e238a46700a6f910436a60bc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants