596555 - Sync page-worker implementation with content-symbiont

Status: RESOLVED FIXED

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

Comment 1

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Sinc page-worker implementation with changes in symbiont

Comment 2

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Sync page-worker implementation with changes in symbiont

Comment 3

[Dietrich Ayala (:dietrich)]

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.

Comment 4

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

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!

Comment 5

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

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

Comment 6

[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".