841606 - add self.isPrivate() to content scripts

Status: RESOLVED WONTFIX

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

Description

[Erik Vold [:erikvold] (please needinfo? me)]

It's a bit of a hassle of content scripts to figure out if they are on a private window or now without something hooked on to the self object.  This would make that use case much easier.

Comment 1

[Erik Vold [:erikvold] (please needinfo? me)]

Irakli you should look at this proposed api change.

Comment 2

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

Is proposed API in a bug title ? Which is addition of `isPrivate` method to the `self` free variable exposed to content scripts ? Assuming that is a case, please see few of my thoughts on that subject:

- It's little unfortunate to have `isPrivate` be a function everywhere except content scripts where it's a method. I think that is confusing, but as we don't have modules for content scripts I don't think we have any better option so far.

- It also feels like in content script it can be just a constant (property) rather than method, cause as far as I can tell it will never change it's value.

- Could you elaborate or point me where it's being discussed, why do we need such a thing in content scripts ? I assume in order to re-use same content script for private windows and non private ones, but is it really worth it ? Maybe one could just create diff pageMod instance for private stuff and diff for non private stuff and everything is obvious then ?

- In content scripts we already do have `self.options`, maybe we should just provide default option `isPrivate` that is either true or false depending on window it's attached to ?

Comment 3

[Erik Vold [:erikvold] (please needinfo? me)]

One of these two places beolow should be updated to include the isPrivate boolean.  I figure the former is best, so that we don't have special cases for the content script `options` json.

https://github.com/mozilla/addon-sdk/blob/master/lib/sdk/content/content-worker.js#L233-L238

https://github.com/mozilla/addon-sdk/blob/master/lib/sdk/content/worker.js#L177-L180

The test in https://github.com/mozilla/addon-sdk/blob/master/test/test-page-mod.js#L694-L718 named testContentScriptOptionsOption is a good test example.

darkowlzz expressed interest in taking this one on.

Comment 4

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #2)
> Is proposed API in a bug title ? Which is addition of `isPrivate` method to
> the `self` free variable exposed to content scripts ? Assuming that is a
> case,

yes it is.

> - Could you elaborate or point me where it's being discussed, why do we need
> such a thing in content scripts ? I assume in order to re-use same content
> script for private windows and non private ones, but is it really worth it ?
> Maybe one could just create diff pageMod instance for private stuff and diff
> for non private stuff and everything is obvious then ?

Yes, this is just an idea I had to make it easier for page-mods to know if they should behave differently.  A `private` include option for PageMods sounds good, but the point is to enable reuse of the same script, and the `private` flag would not help with that afaict.  Best one could do is include an extra script file with `private = true` basically manually providing what I'm describing above.

I think a `private` flag is a good idea still, maybe for another bug?

Comment 5

[Erik Vold [:erikvold] (please needinfo? me)]

Hmm I'm thinking that the PageMod content script can't actually do anything with the private data, except pass it to the main script, so the filter should really be in the main script, where there is already access to a `isPrivate()` method..

So this doesn't seem necessary to me anymore..

Sorry darkowlzz :(