-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
runtime.getURL fixes #35136
runtime.getURL fixes #35136
Conversation
Preview URLs
External URLs (1)URL:
(comment last updated: 2024-08-01 02:43:40) |
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.
LGTM thanks @rebloor
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.
@rebloor Could you open a new PR to address the issues from this review?
@@ -58,6 +58,8 @@ This article provides information about the changes in Firefox 130 that affect d | |||
|
|||
## Changes for add-on developers | |||
|
|||
- {{WebExtAPIRef("runtime.getURL")}} (and the deprecated {{WebExtAPIRef("extension.getURL")}}) have been updated to prefix the supplied path string with the extension's `moz-extension:` origin (in line with the behavior of Chrome). In addition, {{WebExtAPIRef("runtime.getURL")}} no longer strips `/.`, `./`, and `../.` strings from the input URL ([Firefox bug 1795082](https://bugzil.la/1795082)). |
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 description here does not highlight the difference in comparison with the previous implementation.
A better description is to write that the extension origin is now always prepended to the path, without further normalization. Previously, when an absolute URL was provided instead of a relatively URL, that absolute URL was returned. "Without further normalization" captures changes like "/../" and "/./"
@@ -26,7 +26,7 @@ browser.runtime.getURL( | |||
|
|||
### Return value | |||
|
|||
`string`. The fully-qualified URL to the resource. | |||
`string`. The fully-qualified URL to the resource. For example, `moz-extension://e15cdd35-f9b7-4786-a39d-7d801bd2709e/help/open-in-container.html`. |
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.
Let's drop this example. It is not obvious from just this description how the URL is derived from the input.
Furthermore, the code example below already offers a comment showing the format of the return value in Firefox specifically.
`string`. The fully-qualified URL to the resource. For example, `moz-extension://e15cdd35-f9b7-4786-a39d-7d801bd2709e/help/open-in-container.html`. | |
`string`. The fully-qualified URL to the resource within the extension. |
Description
Adds a release note for the changes made in Bug 1795082 browser.runtime.getURL() may return a non-extension URL and a clarifying example in runtime.getURL())