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

Add usage notes to OffscreenCanvas.transferToImageBitmap. #28019

Merged
merged 4 commits into from
Jul 20, 2023

Conversation

kenrussell
Copy link
Contributor

@kenrussell kenrussell commented Jul 18, 2023

Description

Add usage notes to OffscreenCanvas.transferToImageBitmap regarding the ImageBitmap's lifetime.
Reference these notes from the top-level OffscreenCanvas documentation.

Motivation

Some web developers have noted that transferToImageBitmap can be confusing and lead to application bugs; crbug.com/1457212 is one recent example. Add more details to the API doc describing the necessity to either
consume or close the ImageBitmap this API creates, and mention an alternative.

Additional details

crbug.com/1457212 contains a minimized test case from a Chromium customer which was incorrect and which
led to application instability. The customer noted that they used MDN's OffscreenCanvas documentation as a
primary source of information, and requested that the documentation be expanded with this information.

Related issues and pull requests

None

Some web developers have noted that transferToImageBitmap can be
confusing and lead to application bugs; crbug.com/1457212 is one
recent example. Add a section describing the necessity to either
consume or close the ImageBitmap this API creates, and mention an
alternative.
@kenrussell kenrussell requested a review from a team as a code owner July 18, 2023 01:10
@kenrussell kenrussell requested review from sideshowbarker and removed request for a team July 18, 2023 01:10
@github-actions github-actions bot added the Content:WebAPI Web API docs label Jul 18, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 18, 2023

Preview URLs

(comment last updated: 2023-07-20 22:23:34)

Copy link
Member

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

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

Why not place this in the transferToImageBitmap page? Should be more immediately discoverable.

@kenrussell
Copy link
Contributor Author

Why not place this in the transferToImageBitmap page? Should be more immediately discoverable.

Thanks for the suggestion - have moved the new text there. Added a mention in the top-level docs, since people skimming just OffscreenCanvas need to read this note.

@kenrussell kenrussell changed the title Add "Important Notes" to OffscreenCanvas. Add usage notes to OffscreenCanvas.transferToImageBitmap. Jul 18, 2023
@kenrussell
Copy link
Contributor Author

@Josh-Cena @sideshowbarker would you be able to (re-)review this? We received feedback from a web developer that they'd like these clarifications. Thanks.

@sideshowbarker sideshowbarker merged commit a384997 into mdn:main Jul 20, 2023
@kenrussell
Copy link
Contributor Author

Thank you!

@sideshowbarker
Copy link
Collaborator

Cheers — it’s really great to see a change go in like this one that directly addresses some specific developer frustration/confusion

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