-
Notifications
You must be signed in to change notification settings - Fork 22.6k
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
Bug 1943050 user scripts API - move legacy content #37975
Bug 1943050 user scripts API - move legacy content #37975
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The top-level userScripts page still exists. Did you intentionally keep it?
@Rob--W Yes. I saw this as a move of all the existing content so that if someone wanted it, they had the convenience of being able to find it from the sidebar menu. Also, there wouldn't be a convenient redirect for |
I think that we are talking about different things. By top-level userScripts page still existing, I am referring to the fact that this PR kept the original content at https://pr37975.content.dev.mdn.mozit.cloud/en-US/docs/Mozilla/Add-ons/WebExtensions/API/userScripts while the PR moved most of the other pages without keeping the original. I don't mind keeping the original if it helps with the transition, especially if we are going to land the PRs shortly after each other. I asked whether it was intentional to make sure that it was in fact intentional, and that you are not going to be surprised about the existing file when you write the new docs. |
@Rob--W I guess I'm still thinking of this as one atomic change, moving legacy content to a new folder and adding the new API content. So I don't expect to have any issues. Also, I wasn't sure if you would want to put a redirect in place temporarily if this PR was merged ahead of one for the new API content. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good. Most of this review is just polish.
@@ -6123,7 +6123,11 @@ | |||
/en-US/docs/Mozilla/Add-ons/WebExtensions/API/proxy/registerProxyScript /en-US/docs/Mozilla/Add-ons/WebExtensions/API/proxy | |||
/en-US/docs/Mozilla/Add-ons/WebExtensions/API/proxy/unregister /en-US/docs/Mozilla/Add-ons/WebExtensions/API/proxy | |||
/en-US/docs/Mozilla/Add-ons/WebExtensions/API/tabs/moveInSuccession() /en-US/docs/Mozilla/Add-ons/WebExtensions/API/tabs/moveInSuccession | |||
/en-US/docs/Mozilla/Add-ons/WebExtensions/API/userScripts/RegisteredUserScript/RegisteredUserScript.unregister() /en-US/docs/Mozilla/Add-ons/WebExtensions/API/userScripts/RegisteredUserScript/unregister | |||
/en-US/docs/Mozilla/Add-ons/WebExtensions/API/userScripts/RegisteredUserScript/RegisteredUserScript.unregister() /en-US/docs/Mozilla/Add-ons/WebExtensions/API/userScripts_legacy/RegisteredUserScript/unregister |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/en-US/docs/Mozilla/Add-ons/WebExtensions/API/userScripts/RegisteredUserScript/RegisteredUserScript.unregister() /en-US/docs/Mozilla/Add-ons/WebExtensions/API/userScripts_legacy/RegisteredUserScript/unregister | |
/en-US/docs/Mozilla/Add-ons/WebExtensions/API/userScripts/RegisteredUserScript/unregister /en-US/docs/Mozilla/Add-ons/WebExtensions/API/userScripts_legacy/RegisteredUserScript/unregister |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dotproto redirects are set automatically and we're specifically told not to edit them manually, see https://github.com/mdn/content/blob/main/CONTRIBUTING.md#moving-documents
> [!WARNING] | ||
> This is documentation for the legacy `userScripts` API. It's available in Firefox for Manifest V2. For functionality to work with user scripts in Manifest V3 see the new {{WebExtAPIRef("userScripts")}} API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible (and a good idea) to move this block into a reusable fragment that we can include in each page? (I may have asked something like this before on a previous issue. If so, apologies for the repetition.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I'm aware, banners have to be coded as a macro (https://developer.mozilla.org/en-US/docs/MDN/Writing_guidelines/Page_structures/Banners_and_notices). If there is another option, it's not obviously documented.
Even if the process was simple, as the banners here are static and will not be used ad hoc, the benefit of including a reusable fragment seemed limited.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NOTE: This file was renamed to files/en-us/mozilla/add-ons/webextensions/api/userscripts_legacy/registereduserscript/unregister/index.md
(included below).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
files/en-us/mozilla/add-ons/webextensions/api/userscripts/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/userscripts_legacy/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/manifest.json/user_scripts/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/manifest.json/user_scripts/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/manifest.json/user_scripts/index.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 42 should be updated to add a space before the user_scripts
link.
- - : An event available to the API script, registered in[`"user_scripts"`](/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/user_scripts), that execute before a user script executes. Use it to trigger the export of the additional APIs provided by the API script, so they are available to the user script.
+ - : An event available to the API script, registered in [`"user_scripts"`](/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/user_scripts), that execute before a user script executes. Use it to trigger the export of the additional APIs provided by the API script, so they are available to the user script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line will go when the page is updated to add the new API docs.
files/en-us/mozilla/add-ons/webextensions/work_with_contextual_identities/index.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Simeon Vincent <[email protected]>
Description
Initial PR for addressing the documentation needs of Bug 1943050 Enable userScripts API by default. This PR moves the documentation for the
userScript
API available in Manifest V2 to a new_legacy
folder and updates cross-references. Also, include appropriate updates to theuser_script
manifest key documentation.Related issues and pull requests
Addition of related browser compatibility data in mdn/browser-compat-data#25857