Closed
Bug 964624
Opened 10 years ago
Closed 10 years ago
Write API reference documentation for ui/frame
Categories
(Add-on SDK Graveyard :: Documentation, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: wbamberg, Assigned: wbamberg)
Details
Attachments
(3 files, 1 obsolete file)
We need documentation for the ui/frame component.
Attachment #8366460 -
Flags: review?(rFobic)
Comment 2•10 years ago
|
||
> Add HTML content to a Firefox browser toolbar. With this module you can create content for a Firefox toolbar using HTML, CSS and JavaScript. I would like to emphasize the fact that in nutshell it's just an HTML iframe & that while currently it can be added to Toolbar only we plan on allowing adding it to other places as well. So I'd phrase this as: This module let you create HTML iframes (using bundled HTML, CSS and JS) that can be added to a designated areas of Firefox UI. > This module exports the Frame constructor. A Frame is designed to be used in conjunction with a Toolbar: it specifies HTML content to be hosted inside the toolbar. You create a Frame, then supply it to the Toolbar's constructor, and the content is then displayed inside the toolbar. Maybe something like: This module exports the Frame constructor, which can be used to create frame components that can be added to a designated UI areas. Right now it can be used in conjunction with a Toolbar, by providing it in a set of item to be hosted inside the toolbar. > With other SDK modules that interact with web content, the add-on code specifies "content scripts" to attach to the content. With Frame you don't do this. Instead, you include scripts directly from the frame's HTML content, as with a normal web page. Not sure it's worth bringing up content scripts here, as I imagine it may only confuse people. If we do bring them up then it maybe better to explain why here we go about it differently, which is because frame document is coming from add-on itself and there for we trust it enough to let it communicate with privileged code directly. > * sending messages from the main add-on code to a single instance of a frame, attached to a single browser window I'd replace "single browser window" with "specific browser window". > The data property of the event is the message payload: I think it's worth pointing out that it's a JSON copy and that only JSON'able data can be passed over. > Globals I'm not sure that is a best section name, since `Frame` is not a global in SDK. I would rather called it Exports. > id While this property is indeed present right now, we're planning on changing it to `name` instead. Id property will be still present on instances, but will be unique for the Firefox as whole not just per add-on. > Send a message to scripts loaded into the frame. This will send the message to all instances of the frame, across all browser windows I thin this phrasing maybe confusing. I'd prefer to say that message is sent to all view port documents (one per browser window) of the frame. > target : string > Specifies the target of the message, as in window.postMessage(). This parameter is currently not used, and you can just specify "*" here. This should be `targetOrigin` and I would skip the part about not being used. In fact it's always better to use `event.origin` here or some concrete value. But as we don't have add-on origin yet, we kind of stuck with `*` for the messages that aren't replies. > pong In the examples with `pong` function I'd changed "*" to `e.origin`: e.source.postMessage("pong", e.origin); That is more secure & there for recommended approach. > load I would not advertise event.source.id or event.source.ownerID as they are implementation details that can change. Only thing that is guaranteed is that event.source.postMessage can be used to send messages back to where they came from.
Comment 3•10 years ago
|
||
Comment on attachment 8366460 [details]
mdn.html
Looks mostly good, just putting - only to make sure that we don't document
unstable API of `event.source` and provide examples with more recommended
`e.postMessage(data, e.origin)` pattern instead.
Attachment #8366460 -
Flags: review?(rFobic) → review-
Comment 4•10 years ago
|
||
> Like a panel, a toolbar's content is specified using HTML. Unlike a panel, a toolbar:
I would not say it's content is HTML, since it already can contain all our buttons & in a future
we plan to add even more stuff.
Other than that everything looks fine to me. It also maybe worth mentioning buttons though since
support for them has landed as well.
Updated•10 years ago
|
Attachment #8368962 -
Flags: review?(rFobic) → review+
Comment 5•10 years ago
|
||
When Will was working on docs we discovered some mismatches between what was in JEP's and what's in implementation. This change updates JEPs to match what ended up implemented. In addition this update covers rename of `options.id` to `options.name`.
Attachment #8381559 -
Flags: review?(zer0)
Attachment #8381559 -
Flags: feedback?(wbamberg)
Priority: -- → P1
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #2) > > Add HTML content to a Firefox browser toolbar. With this module you can create content for a Firefox toolbar using HTML, CSS and JavaScript. > > > > Globals > > I'm not sure that is a best section name, since `Frame` is not a global in > SDK. I would rather called it > Exports. Well, this is the heading given to module-level exports across all the API reference docs, so I don't want to change it for just this one. And I did ask for feedback about this structure when I ported the docs to MDN, and noone brought this up, so I don't particularly want to change them all now. If I did, though, I'm not sure "Exports" is better, since the whole API is exported, by definition. "Module-level exports" might be better. > > > id > > While this property is indeed present right now, we're planning on changing > it to `name` instead. Id property will be still present on instances, but > will be unique for the Firefox as whole not just per > add-on. Is this happening in Firefox 29? If not, in which release will it change to "name"? Will the specification of it still be the same? Will "id" still be public and settable as a constructor option? Is there a bug I can track for this? > > > Send a message to scripts loaded into the frame. This will send the message to all instances of the frame, across all browser windows > > I thin this phrasing maybe confusing. I'd prefer to say that message is sent > to all view port documents > (one per browser window) of the frame. > > > > target : string > > Specifies the target of the message, as in window.postMessage(). This parameter is currently not used, and you can just specify "*" here. > > This should be `targetOrigin` and I would skip the part about not being > used. In fact it's always better > to use `event.origin` here or some concrete value. But as we don't have > add-on origin yet, we kind of > stuck with `*` for the messages that aren't replies. > > > > pong > > In the examples with `pong` function I'd changed "*" to `e.origin`: > e.source.postMessage("pong", e.origin); > > That is more secure & there for recommended approach. To be clear about this though, do you need to use "*" in these cases: * if you're messaging from the frame script to the add-on? * if you're messaging from the add-on to all "view port documents" (all windows)? * if you're initiating conversation with a particular frame script?
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(rFobic)
Comment 7•10 years ago
|
||
(In reply to Will Bamberg [:wbamberg] from comment #6) > (In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment > #2) > > > Add HTML content to a Firefox browser toolbar. With this module you can create content for a Firefox toolbar using HTML, CSS and JavaScript. > > > > > > > Globals > > > > I'm not sure that is a best section name, since `Frame` is not a global in > > SDK. I would rather called it > > Exports. > > Well, this is the heading given to module-level exports across all the API > reference docs, so I don't want to change it for just this one. And I did > ask for feedback about this structure when I ported the docs to MDN, and > noone brought this up, so I don't particularly want to change them all now. > If I did, though, I'm not sure "Exports" is better, since the whole API is > exported, by definition. "Module-level exports" might be better. > > > > > > id > > > > While this property is indeed present right now, we're planning on changing > > it to `name` instead. Id property will be still present on instances, but > > will be unique for the Firefox as whole not just per > > add-on. > > Is this happening in Firefox 29? If not, in which release will it change to > "name"? This has landed & will be uplifted to Aurora tomorrow. > Will the specification of it still be the same? I'm not sure what you mean, but I have updated JEP that reflect that change & has full up to date specification. > Will "id" still be public and settable as a constructor option? Is there a bug I can track for > this? No `id` won't be settable via option as it needs to be unique across all add-ons, there for add-on can provide `name` instead which is unique with in that add-on and we'll use that to generate `id` internally that is unique across all add-ons. Generated `id` is exposed via `frame.id` property. `options.name` isn't exposed anywhere it's just a hint really. > > > > > Send a message to scripts loaded into the frame. This will send the message to all instances of the frame, across all browser windows > > > > I thin this phrasing maybe confusing. I'd prefer to say that message is sent > > to all view port documents > > (one per browser window) of the frame. > > > > > > > target : string > > > Specifies the target of the message, as in window.postMessage(). This parameter is currently not used, and you can just specify "*" here. > > > > This should be `targetOrigin` and I would skip the part about not being > > used. In fact it's always better > > to use `event.origin` here or some concrete value. But as we don't have > > add-on origin yet, we kind of > > stuck with `*` for the messages that aren't replies. > > > > > > > pong > > > > In the examples with `pong` function I'd changed "*" to `e.origin`: > > e.source.postMessage("pong", e.origin); > > > > That is more secure & there for recommended approach. > > To be clear about this though, do you need to use "*" in these cases: > > * if you're messaging from the frame script to the add-on? That's only if frame initiates communication, but if add-on does instead frame script can just reply as: `event.source.postMessage(message, event.origin)` > * if you're messaging from the add-on to all "view port documents" (all > windows)? In that case that argument is actually not really necessary, but if you'd want to specify it I'd suggest to use frame.url instead since it limits it to a url that you've loaded. > * if you're initiating conversation with a particular frame script? In such case add-on can reply as event.source.postMessage(message, event.origin) instead. BTW JEP I have posted here and requested your feedback on has all the examples reflecting it, so it may be worth taking a look at it ;)
Flags: needinfo?(rFobic)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8381559 [details] [review] Updated JEP Yes Irakli, that makes a lot of sense :).
Attachment #8381559 -
Flags: feedback?(wbamberg) → feedback+
Assignee | ||
Comment 9•10 years ago
|
||
Here's another attempt at ui/frame.
Attachment #8366460 -
Attachment is obsolete: true
Attachment #8389534 -
Flags: review?(rFobic)
Comment 10•10 years ago
|
||
Comment on attachment 8381559 [details] [review] Updated JEP r+ with the typos fixed.
Attachment #8381559 -
Flags: review?(zer0) → review+
Comment 11•10 years ago
|
||
Comment on attachment 8389534 [details] [diff] [review] mdn-ui-frame.html It looks good, with except that all the `id` options should be renamed to `name` instead. I would also really like updated images to avoid shoving ugly scrollbars. I don't think we need another review on this, putting r- only to indicate that some followup work is required.
Attachment #8389534 -
Flags: review?(rFobic) → review-
Assignee | ||
Comment 12•10 years ago
|
||
Thanks Irakli. I've made those changes, so I'll close this bug now.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•