1236500 - browser.tabs.executeScript should resolve file URLs based on the add-on baseURI

Status: RESOLVED FIXED

(Developer Documentation Graveyard :: Add-ons, defect)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

browser.tab.executeScript currently resolve the file URLs based on the current page URI, but it should instead resolve it based on the add-on baseURI.

Follows an example add-on which presents the above issues:

- https://github.com/mdn/webextensions-examples/tree/master/beastify
  - https://github.com/mdn/webextensions-examples/issues/21

The above add-on works correctly when it is loaded on a "Chromium based" browser (and it fails if we try to apply the suggested workaround)

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: 0001-Bug-1236500-browser.tabs.executeScript-should-resolv.patch

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: 0001-Bug-1236500-browser.tabs.executeScript-should-resolv.patch

In this new version of the patch there are both the one-line fix and a new test case for it.

Comment 3

[Kris Maglione [:kmag]]

For other APIs, we've decided we should consistently resolve relative to the context URI, even if it doesn't match Chrome's behavior. I'd rather do the same for this API as well, since I think relative URIs make more sense here.

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Kris Maglione [:kmag] from comment #3)
> For other APIs, we've decided we should consistently resolve relative to the
> context URI, even if it doesn't match Chrome's behavior. I'd rather do the
> same for this API as well, since I think relative URIs make more sense here.

I would agree with you but I'm a bit concerned about the confusion and frustration that this kind of differences could generate in the developers that are converting their add-on from chrome or that wants to create cross-browser add-ons.

At least we should change the goal of this issue into a paragraph for the "WebExtensions - Chrome incompatibilities" doc page on MDN (https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Chrome_incompatibilities) and point there every add-on developer which needs a workaround for this issue.

If it's possible and reasonable, I'd like to help webextensions add-ons which can work cross-browser to include the minimum amount of annoying workarounds like:

     if (/* fanciful method to detect if you are running of Chrome or Firefox */) {
       ...
     } else {
       ...
     }

The simpler and clearer workaround, which works on both Firefox and Chrome, seems to be "using an explicit absolute url", e.g.:

     chrome.tabs.executeScript({ file: "/path/to/script.js" });

Comment 5

[Kris Maglione [:kmag]]

(In reply to Luca Greco [:rpl] from comment #4)
> I would agree with you but I'm a bit concerned about the confusion and
> frustration that this kind of differences could generate in the developers
> that are converting their add-on from chrome or that wants to create
> cross-browser add-ons.

It should definitely be listed, yes.

> The simpler and clearer workaround, which works on both Firefox and Chrome,
> seems to be "using an explicit absolute url", e.g.:
>
>      chrome.tabs.executeScript({ file: "/path/to/script.js" });

Agreed, that would probably be worth mentioning.

Comment 6

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Kris Maglione [:kmag] from comment #5)
> (In reply to Luca Greco [:rpl] from comment #4)
> > I would agree with you but I'm a bit concerned about the confusion and
> > frustration that this kind of differences could generate in the developers
> > that are converting their add-on from chrome or that wants to create
> > cross-browser add-ons.
> 
> It should definitely be listed, yes.
> 
> > The simpler and clearer workaround, which works on both Firefox and Chrome,
> > seems to be "using an explicit absolute url", e.g.:
> >
> >      chrome.tabs.executeScript({ file: "/path/to/script.js" });
> 
> Agreed, that would probably be worth mentioning.

The documentation page fragment at: 
- https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/executeScript#Parameters
- https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/ExtensionTypes/InjectDetails

seems to be generated (at least partially) from this schema file:
- https://dxr.mozilla.org/mozilla-central/source/toolkit/components/extensions/schemas/extension_types.json#47

How about applying to it a change similar to the following?

   - "file": {"type": "string", "optional": true, "description": "JavaScript or CSS file to inject."},
   + "file": {"type": "string", "optional": true, "description": "JavaScript or CSS file to inject. <br><br><b>Warning:</b><br>The <code>file</code> URLs are relative to the current page URL on Firefox, See <a href=\"https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Chrome_incompatibilities\">WebExtensions - Chrome incompatibilities</a> to get more information about cross-browser workarounds."},

My main concern about the above change to the schema is the risk to overwrite it and losing the additional note on the next schema update.

Comment 7

[Kris Maglione [:kmag]]

I don't think it should be a bold warning. It would be nice to update the description to say that URLs are resolved relative to the current page, and note that Chrome does it differently.

My understanding is that it's safe to modify the wiki pages now that they're up on production, but I'm not sure.

Comment 8

[Will Bamberg [:wbamberg]]

(In reply to Kris Maglione [:kmag] from comment #7)
> I don't think it should be a bold warning. It would be nice to update the
> description to say that URLs are resolved relative to the current page, and
> note that Chrome does it differently.
> 
> My understanding is that it's safe to modify the wiki pages now that they're
> up on production, but I'm not sure.

Yes, the plan was to use the automated import to give us a skeleton structure and a starting point, but then to improve and update the pages manually (it's possible that we might try to devise a way to do future automated updates without splatting manual edits, but that's not in the plan right now).

Having said that, we only just pushed the pages yesterday, and we might want to do another automated push after we get some feedback on the structure, but before too many manual edits have been made. So I don't want to encourage massive manual edits for the next couple of weeks.

But let's update the page to talk about this anyway.

And documenting the path for executeScript would be really helpful :).

Comment 9

[Luca Greco [:rpl] [:luca] [:lgreco]]

In the mean time I'd like to add a reference to how executeScript resolve file URLs in the "Chrome incompatibilities MDN page"[1]:

By introducing a new item in the "Additionally, Firefox currently" list of the "tabs" section [2]:

- "executeScript()" resolves file URLs as relative to thee URL of the current page,
  in cross-browser add-ons you should use absolute URLs, e.g. 

    chrome.tabs.executeScript({ file: "/path/to/script.js" });

It would be even better if we add more about relative URLs in the "Miscellaneous incompatibilities" section [3]
(where it would be better if we can put the full story about of when the URL is resolved as relative to the extension base URL, e.g. "chrome.runtime.getURL()" and when is resolved as relative to current page URL, e.g. "chrome.tabs.executeScript()"), e.g.:

- Firefox resolves most of the URLs parameters as relative to the current page URL
  where Chrome most of the times resolves them as relative to the extension base URL,
  in cross-browser add-ons you should use absolute URLs, e.g. 

    // resolved as relative to the current page URL 
    chrome.tabs.executeScript({ file: "/path/to/script.js" });

    // resolved as relative to the extension base URL
    chrome.runtime.getURL({ file: "path/to/script.js" });

Or by totally reworking the "Miscellaneous incompatibilities"[3] and "CSS"[4] sections:

- Miscellaneous incompatibilities
  - Optional Arguments
    - Chrome always ...
  - Relative URLS
    - In CSS, we resolve URLs...
    - In tabs.executeScript, file URLs are resolved as relative to the current page URL

Will, what is the best option in your opinion?

[1]: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Chrome_incompatibilities
[2]: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Chrome_incompatibilities#tabs
[3]: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Chrome_incompatibilities#Miscellaneous_incompatibilities
[4]: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Chrome_incompatibilities#CSS

Comment 10

[Kris Maglione [:kmag]]

Comment on attachment 8703635 [details] [diff] [review]
0001-Bug-1236500-browser.tabs.executeScript-should-resolv.patch

Review of attachment 8703635 [details] [diff] [review]:
-----------------------------------------------------------------

Clearing the review flag for now, because I'm not sure we want to do this. Please flag me again if it turns out we do.

Comment 11

[Will Bamberg [:wbamberg]]

(In reply to Luca Greco [:rpl] from comment #9)
> In the mean time I'd like to add a reference to how executeScript resolve
> file URLs in the "Chrome incompatibilities MDN page"[1]:
> 
> By introducing a new item in the "Additionally, Firefox currently" list of
> the "tabs" section [2]:
> 
> - "executeScript()" resolves file URLs as relative to thee URL of the
> current page,
>   in cross-browser add-ons you should use absolute URLs, e.g. 
> 
>     chrome.tabs.executeScript({ file: "/path/to/script.js" });
> 
> It would be even better if we add more about relative URLs in the
> "Miscellaneous incompatibilities" section [3]
> (where it would be better if we can put the full story about of when the URL
> is resolved as relative to the extension base URL, e.g.
> "chrome.runtime.getURL()" 

But this isn't an incompatibility, is it? This is the same behavior as Chrome, no? I think it's best to keep this page to documenting incompatibilities, and assume that things not described here are compatible.

and when is resolved as relative to current page
> URL, e.g. "chrome.tabs.executeScript()"), e.g.:
> 
> - Firefox resolves most of the URLs parameters as relative to the current
> page URL
>   where Chrome most of the times resolves them as relative to the extension
> base URL,
>   in cross-browser add-ons you should use absolute URLs, e.g. 
> 
>     // resolved as relative to the current page URL 
>     chrome.tabs.executeScript({ file: "/path/to/script.js" });
> 
>     // resolved as relative to the extension base URL
>     chrome.runtime.getURL({ file: "path/to/script.js" });
> 
> Or by totally reworking the "Miscellaneous incompatibilities"[3] and
> "CSS"[4] sections:
> 
> - Miscellaneous incompatibilities
>   - Optional Arguments
>     - Chrome always ...
>   - Relative URLS
>     - In CSS, we resolve URLs...
>     - In tabs.executeScript, file URLs are resolved as relative to the
> current page URL
> 
> Will, what is the best option in your opinion?
>

I've noted this incompatibility in the "tabs" section, and have done that: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Chrome_incompatibilities#tabs. When we have reference pages for executeScript and insertCSS, I'll write something a bit more expansive on this.

I've also collapsed the "CSS" section into "Miscellaneous".

Let me know if this helps!

Comment 12

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Will Bamberg [:wbamberg] from comment #11)
> But this isn't an incompatibility, is it? This is the same behavior as
> Chrome, no? 

that's right: chrome.runtime.getURL has exactly the same behavior on Chrome

> I think it's best to keep this page to documenting incompatibilities, 
> and assume that things not described here are compatible.

I agree, probably "Chrome incompatibilities" is not the right page for this,
but it would be helpful to explicitly mention in one of the doc pages
which are the different "relative URLs resolution strategies" implemented,
(e.g. relative to the extension base URL, to the page URL or to the file URL),
and give examples about which one the existent API methods use and why.

> I've noted this incompatibility in the "tabs" section, and have done that:
> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/
> Chrome_incompatibilities#tabs. When we have reference pages for
> executeScript and insertCSS, I'll write something a bit more expansive on
> this.

Looks great, thanks!

> I've also collapsed the "CSS" section into "Miscellaneous".

I'm bit concerned that in the new version the "CSS" paragraph is not as much visible as before:

in the previous version, the CSS title of the sub-section make it much more visible and easy to be spotted in the topic list.

> 
> Let me know if this helps!

Absolutely! 
In my opinion we now have enough docs about the topic on MDN and we can close this issue.

Comment 13

[Will Bamberg [:wbamberg]]

> > I've also collapsed the "CSS" section into "Miscellaneous".
> 
> I'm bit concerned that in the new version the "CSS" paragraph is not as much
> visible as before:
> 
> in the previous version, the CSS title of the sub-section make it much more
> visible and easy to be spotted in the topic list.
> 

Fair enough. But I don't think "CSS" made sense as an H3. So I've made it a subheading under "Miscellaneous".