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

Documenting renderBlockingStatus in Resource Timing #19325

Merged
merged 17 commits into from
Nov 2, 2022

Conversation

abinpaul1
Copy link
Contributor

Summary

New field renderBlockingStatus in PerfomanceResourceTiming to indicate the render-blocking nature of a resource. The attribute takes away the burden of having to rely on complex heurestics to determine which resources were actually render-blocking and provides a direct signal from the browser.

Explainer : https://github.com/abinpaul1/resource-timing/blob/render-blocking-status-explainer/Explainer/Render_Blocking_Status.md

Spec changes : w3c/resource-timing#327

Motivation

Adding documentation for new attributerenderBlockingStatus introduced to Resource Timing. Adding sample code to demonstrate usage of the attribute.

Supporting details

CR Bug : https://bugs.chromium.org/p/chromium/issues/detail?id=1337256
Chrome status entry : https://chromestatus.com/feature/5166965277589504

Related issues

Metadata

  • Adds a new document
  • Rewrites (or significantly expands) a document
  • Fixes a typo, bug, or other error

@abinpaul1 abinpaul1 requested a review from a team as a code owner August 9, 2022 13:56
@abinpaul1 abinpaul1 requested review from wbamberg and removed request for a team August 9, 2022 13:56
@abinpaul1 abinpaul1 marked this pull request as draft August 9, 2022 13:56
@github-actions github-actions bot added the Content:WebAPI Web API docs label Aug 9, 2022
@bsmth
Copy link
Member

bsmth commented Sep 22, 2022

Hi @abinpaul1, thanks for opening the PR. I'm just checking if you're planning to pick this one up again or how you'd like to proceed. Thanks a lot :)

@abinpaul1
Copy link
Contributor Author

I was thinking I'd have to wait for Chrome 107 to ship which would be the first browser release to have this enabled. I'm happy to continue working on this right away. Please feel free to start a review for this and I can adjust it based on feedback or I'm also open to handing it over if thats preferred.

@bsmth
Copy link
Member

bsmth commented Sep 23, 2022

I'd have to wait for Chrome 107 to ship

Thanks a lot, I just had a look through the Chrome bug and it looks like it's expected to ship soon, is that right? Feel free to ping me to have another look! :)

@abinpaul1 abinpaul1 marked this pull request as ready for review September 23, 2022 14:40
@abinpaul1 abinpaul1 requested a review from a team as a code owner September 23, 2022 14:40
@abinpaul1
Copy link
Contributor Author

Yes, stable is expected to ship towards end of October. I am going ahead and marking the PR as ready for review.

@bsmth
Copy link
Member

bsmth commented Sep 23, 2022

OK thanks a lot. There will be some downtime from the team during the next week or so; we will get to this afterwards. I'm happy to pick it up then.

@wbamberg wbamberg removed their request for review September 23, 2022 21:58
@bsmth bsmth self-assigned this Oct 5, 2022
@abinpaul1
Copy link
Contributor Author

abinpaul1 commented Oct 16, 2022

@bsmth I'm not able to get around an error when I add this new document for renderBlockingStatus. I am trying to keep it similar to how all the other fields in Resource Timing are added, but I get this error.

MacroBrokenLinkError from domxref [line 112:7] 
Explanation: /en-US/docs/Web/API/PerformanceResourceTiming/renderBlockingStatus does not exist `

Mainly the document isnt being picked up for some reason. Navigating to /en-US/docs/Web/API/PerformanceResourceTiming/renderBlockingStatus gives page not found
I feel I'm misssing something to get the links to work correctly

@bsmth
Copy link
Member

bsmth commented Oct 17, 2022

I feel I'm misssing something to get the links to work correctly

It could be a case-sensitivity thing on the parent directory for the document. I can take a look shortly

…atus/index.md to files/en-us/web/api/performanceresourcetiming/renderblockingstatus/index.md
@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2022

Preview URLs

(this comment was updated 2022-11-02 10:44:00.349468)

@bsmth
Copy link
Member

bsmth commented Oct 17, 2022

@abinpaul1 this is fixed now in ac79f03, the parent dir is case-sensitive

@abinpaul1
Copy link
Contributor Author

Cool! Thanks

@bsmth
Copy link
Member

bsmth commented Oct 18, 2022

I tested in Chrome 108.0.5355.0 and works as expected. This should be on-hold until 107 stable release, but I think we can publish it then. We will need corresponding browser compat data for this first.

@abinpaul1
Copy link
Contributor Author

I've opened a PR for adding the compat data : mdn/browser-compat-data#18046

@bsmth
Copy link
Member

bsmth commented Oct 19, 2022

I've opened a PR for adding the compat data : mdn/browser-compat-data#18046

oh nice, you beat me to it 🙌🏻

@abinpaul1
Copy link
Contributor Author

Hey @bsmth, Are we good to merge this?

@bsmth
Copy link
Member

bsmth commented Oct 27, 2022

Hi @abinpaul1, I think the corresponding BCD PR has to be merged first. Once that's landed I think we're ready to go here.

@bsmth
Copy link
Member

bsmth commented Oct 28, 2022

BCD for this is merged, this PR can be merged when this is deployed (likely today).

@Josh-Cena
Copy link
Member

This should be mergeable now?

Copy link
Member

@bsmth bsmth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @abinpaul1, going to merge shortly

@bsmth bsmth requested a review from a team as a code owner November 2, 2022 10:29
@bsmth bsmth merged commit cdaead4 into mdn:main Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants