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

Document changes to action.openPopup API #22267

Merged
merged 4 commits into from
Aug 29, 2023
Merged

Conversation

oliverdunk
Copy link
Contributor

Description

Updates the documentation for the action.openPopup API following changes across browsers, including the Firefox changes in https://phabricator.services.mozilla.com/D139796.

I've removed the note about user gesture requirements from the main body since this does not apply in any other browser (see https://docs.google.com/document/d/1xzuw1dMuf5Lub8UPME2zjxyp3nCKXk-3jtSY2kdI-FQ/edit?usp=sharing for Chrome) or Safari. Instead I've added it as a compatibility note in mdn/browser-compat-data#18202.

All browsers now also support a windowId parameter so I've added documentation for that.

Motivation

I've been working on this API recently and want to make sure the documentation is an up to date point of reference.

Additional details

Related issues and pull requests

Depends on: mdn/browser-compat-data#18202

@oliverdunk oliverdunk requested a review from a team as a code owner November 13, 2022 14:07
@oliverdunk oliverdunk requested review from rebloor and removed request for a team November 13, 2022 14:07
@github-actions github-actions bot added Content:Other Any docs not covered by another "Content:" label Content:WebExt WebExtensions docs labels Nov 13, 2022
Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

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

We found quite some interesting behaviors and edge cases during the development of this feature. Did you already check and rule out their relevance for the documentation?

Copy link
Contributor

@rebloor rebloor left a comment

Choose a reason for hiding this comment

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

Changes plus suggested changes, good with me. Thanks @oliverdunk

@github-actions
Copy link
Contributor

github-actions bot commented Nov 15, 2022

@Josh-Cena
Copy link
Member

Pinging @oliverdunk there are unaddressed reviews.

@oliverdunk
Copy link
Contributor Author

Sorry for the delay on this. I haven't had a chance to look over the feedback yet but will try to do it soon.

@rebloor
Copy link
Contributor

rebloor commented Aug 28, 2023

@oliverdunk let me know if you don't have time to complete this, I'd be happy to finish it up

@github-actions github-actions bot removed the Content:Other Any docs not covered by another "Content:" label label Aug 28, 2023
@oliverdunk
Copy link
Contributor Author

@oliverdunk let me know if you don't have time to complete this, I'd be happy to finish it up

Thanks for the ping! I've tried to address the feedback - feel free to also make any changes on the branch that make sense.

We found quite some interesting behaviors and edge cases during the development of this feature. Did you already check and rule out their relevance for the documentation?

We could probably add a lot more of the different inconsistencies as mentioned in w3c/webextensions#160 (comment). I've left that for now since it seems like a rabbit hole to try and document (and hopefully we'll fix some of them) but happy for anyone else to add on :)

@rebloor
Copy link
Contributor

rebloor commented Aug 29, 2023

@Rob--W I can't merge this while it still has your request for changes on it. The specific items you mentioned have been addressed. As this is now nine months old, perhaps we can address any need for documentation of the nuances separately?

Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

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

Fine by me!

@rebloor rebloor merged commit 920293f into mdn:main Aug 29, 2023
6 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Content:WebExt WebExtensions docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants