588225 - require content of Panel API to be specified as URL

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: implements change

We've talked about changing the way API consumers specify content to load into content frames to prevent mistakes in which presumed HTML content is treated as a URL.  The new Panel API is a good time to introduce a new model.  Here's one shot at it.

This patch replaces the content property with a contentURL property that only accepts a URL.  With this patch, there is no question about what content is loaded into a panel, as it is always the content located at the specified URL.

One can still load HTML content into a panel with this patch applied, one must just construct a data: URL from the content oneself, i.e.:

  let url = "data:text/html," + encodeURIComponent("[HTML content]");

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2: docs, example fixes

Erm, forgot I needed to touch docs and the example app too.

Comment 2

[Brian Warner [:warner :bwarner]]

Comment on attachment 466841 [details] [diff] [review]
patch v2: docs, example fixes

Implementation and tests look good. I'd like to see the docs/examples make it a bit more clear that the argument can either be a string or a URL object.. in particular, I'd like to see an example of using require("self").data.url("foo.html") .

Possibly out-of-scope: there was a recent bug in which the URL instance returned by a tab.getLocation was not accepted by an interface which claimed to accept URL instances.. it might be good to make it clear that things like that are supposed to work too.

+  @prop [contentURL] {URL,string}
+    The URL of the content to load in the panel.

A clear terminology distinction between URL object and string object will help avoid confusion here. Is "URL,string" our convention for "or"? (I don't know to what extent our docstring type markers are parsed further.. maybe {URL object or string}?). Just saying "URL" feels fuzzy.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #2)
> Implementation and tests look good. I'd like to see the docs/examples make it a
> bit more clear that the argument can either be a string or a URL object.. in
> particular, I'd like to see an example of using
> require("self").data.url("foo.html") .

Good point!  I have added one to the Examples section.


> Possibly out-of-scope: there was a recent bug in which the URL instance
> returned by a tab.getLocation was not accepted by an interface which claimed to
> accept URL instances.. it might be good to make it clear that things like that
> are supposed to work too.

Yeah, that's been on my mind.  I'm not sure why it doesn't just work, but let's address that problem separately.


> +  @prop [contentURL] {URL,string}
> +    The URL of the content to load in the panel.
> 
> A clear terminology distinction between URL object and string object will help
> avoid confusion here. Is "URL,string" our convention for "or"? (I don't know to
> what extent our docstring type markers are parsed further.. maybe {URL object
> or string}?). Just saying "URL" feels fuzzy.

Yeah {URL,string} is the convention for "object of type URL or primitive value of type string".  It's a bit obtuse, but it's the current standard, so I've left it as is.  Hopefully we can find some good way to represent such polymorphic properties in the rendered HTML.

Fixed by changeset https://hg.mozilla.org/labs/jetpack-sdk/rev/b723b1b890b6.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".