Sync page-worker implementation with content-symbiont

RESOLVED FIXED

Status

Add-on SDK
General
--
blocker
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: irakli, Assigned: irakli)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
Blocks: 596528
Assignee: nobody → rFobic
No longer blocks: 596528
Blocks: 596528
Created attachment 475514 [details] [diff] [review]
Sinc page-worker implementation with changes in symbiont
Attachment #475514 - Flags: review?
Depends on: 595980
Created attachment 475522 [details] [diff] [review]
Sync page-worker implementation with changes in symbiont
Attachment #475514 - Attachment is obsolete: true
Attachment #475522 - Flags: review?(dietrich)
Attachment #475514 - Flags: review?
Comment on attachment 475522 [details] [diff] [review]
Sync page-worker implementation with changes in symbiont

switching to myk, i'm not familiar enough w/ this code.
Attachment #475522 - Flags: review?(dietrich) → review?(myk)
Blocks: 596524
Comment on attachment 475522 [details] [diff] [review]
Sync page-worker implementation with changes in symbiont

The actual test failure here is caused by the removal of content-symbiont.js, on which page-worker.js depends, from the codebase.  And the simplest, lowest-risk fix for the bug is to re-add that file.

However, in this case I think syncing the page-worker implementation with the recent changes to symbionts is the right thing to do, as it means that we only break the page-worker API for E10S-compatibility one time for developers following along with our regularly bifortnightly releases, rather than breaking it once in 0.8 and again in 0.8.next.  So this patch, although riskier, is the one we should take.


>+  @prop [onReady] {function}
>+    A function callback that will be called when the DOM on the page is ready.
>+    This can be used to know when your Page Worker instance is ready to be used,
>+    and also whenever the page is reloaded or another page is loaded in its
>+    place.

Now that it is no longer possible to access the DOM of the page from the program (and trivial to access it from the content scripts), onReady is no longer useful and should be removed.  Its presence is due to a mistake in my original work to make Page Mods E10S-compatible over in bug 569481.

However, removing onReady is orthogonal to the purpose of this bug, and thus it is not necessary for this patch to do so.  I have filed bug 596754 on the issue.


>+exports.Page = function(options) Page(options)

Nit: semi-colon at end of line.

Otherwise, this looks great, r=myk!
Attachment #475522 - Flags: review?(myk) → review+
Fixed by changeset https://hg.mozilla.org/labs/jetpack-sdk/rev/bb5199283c86.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
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".
Component: Jetpack SDK → General
Product: Mozilla Labs → Add-on SDK
QA Contact: jetpack-sdk → general
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.