Closed Bug 767168 Opened 13 years ago Closed 8 years ago

For page-worker, constructor content script and page scripts are mutually exclusive.

Categories

(Add-on SDK Graveyard :: Documentation, defect, P2)

Product:
Add-on SDK Graveyard
Component:
Documentation
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED INCOMPLETE

People

(Reporter: Gilgeson, Assigned: wbamberg)

Details

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_4) AppleWebKit/536.5 (KHTML, like Gecko) Chrome/19.0.1084.56 Safari/536.5 Steps to reproduce: I am not sure if this scenario is intentional. When I create a new page worker that uses a URL containing content scripts and also try to use the page worker's cotentScript option, only the contentScript option is used and the html's script tags are not executed. If the content script is skipped in the constructor then the script in the html will work. --- var pageWorker = require('page-worker').Page({ contentURL: self.data.url('test.html'), contentScript: "addon.port.emit('msg', 'cs');" }); pageWorker.port.on('msg', function(x) {console.log(x);}); --- <!DOCTYPE html> <html> <head> <script>addon.port.emit('msg', 'html');</script> </head> </html> Actual results: Console prints "cs". Expected results: Console prints "html" then "cs".
According to https://addons.mozilla.org/en-US/developers/docs/sdk/latest/packages/addon-kit/page-worker.html in the Scripting Trusted Page Content section, it vaguely mentions that you have to remove the contentScript(File) part of the worker's constructor, so I think this is actually working as intended. Maybe the documentation could be clearer about this. Will, your thoughts?
If it makes a difference ... I read that section as how you convert from an existing contentScriptFile to an in-page script and not as a mandatory requirement.
I don't think "remove the contentScriptFile option in the Page() constructor" is particularly vague. Let me know how it could be clearer though. > I read that section as how you convert from > an existing contentScriptFile to an in-page script > and not as a mandatory requirement. Yes, that's the intention of that section: "given an add-on that loads trusted content and uses content scripts to access it, there are typically three changes you have to make, if you want to use normal page scripts instead". I wasn't aware, either, that they were mutually exclusive, and if this is deliberate it does need to be explicitly documented.
I apologize for not being clear myself in my statement for how I read the text. Yes, "remove the contentScriptFile option in the Page() constructor" is clear. Just to make sure: I interpreted its meaning as the obvious you must remove this line so you don't have duplicate script injection. Otherwise it would be added once by the constructor and once by the page if not removed. I DO think the section is good at describing the conversion. If it is mutually exclusive a simple note at the end of the section would suffice, at least for me.
Yes, they are exclusive. Sorry if the doc wasn't clear enough about that. Content scripts doesn't make much sense for trusted document. Content scripts are a pattern meaningfull and usefull against untrusted documents, like web documents. So that we would like to encourage people to use `addon.` directly from trusted document and stop using content scripts. Instead we would only use them in page-mod, and eventually other APIs like Panel and Page-Worker but only if the document comes from the web. I think that the main issue is that we didn't provided a safe/stable support for this new feature. So that it can still be documented as new/experimental feature. We would be able to write a more explicit documentation if we promote this feature as a stable, highly suggested one. Having said that, if you think we should really support content script for trusted documents, I can buy into it if you have good usecase for it?
I am not certain that it is useful. Your reasoning seems good to me, and I don't think it is necessary. The only stretch case I can come up with is I want to mix code generated within the addon with code loaded by the page-worker's page. For example, I have a trusted html page already created that relies on some set of functionality not loaded by the page explicitly. I could then use contentScriptFile to load dependency scripts at "start" for the document. I would expect these dependency scripts to exist in the same sandbox as the scripts loaded in-page. I am not sure even that use-case is useful. I am looking at solving this problem (without adding the deps to the page). However, I want to use this in multiple places and not just for page-workers; panel, addon pages loaded directly in tab, etc. So to that end, I was actually exploring the use of page-mod and unsafeWindow to add trusted content on "start" to trusted pages. I mainly see the stretch case only useful for page-workers if the dependencies can change and the addon arbitrates the addition of deps. Thank you all for looking into this. I appreciated the info and help. I am sorry I posted something that was not an actual bug.
(In reply to Jason Stredwick (:Gilgeson) from comment #6) > > Thank you all for looking into this. I appreciated the info and help. I am > sorry I posted something that was not an actual bug. Not at all. At least I didn't know about this before, so I'm glad you raised it. And it is a bug, I think, just a documentation bug, at least to mention that they are in fact mutually exclusive. And as Alex suggested, it would be good to turn all this documentation around and effectively deprecate content scripts for trusted content, but I don't know if we're ready for that yet.
Component: General → Documentation
QA Contact: general → documentation
Assignee: nobody → wbamberg
https://bugzilla.mozilla.org/show_bug.cgi?id=1399562
Status: UNCONFIRMED → RESOLVED
Closed: 8 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.