Functions are logged as "null" in Addon SDK content scripts

RESOLVED FIXED

Status

Add-on SDK
General
P2
normal
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Steve Sobel, Assigned: zer0)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_2) AppleWebKit/537.22 (KHTML, like Gecko) Chrome/25.0.1364.160 Safari/537.22

Steps to reproduce:

I use MutationObserver in my addon, Reddit Enhancement Suite.

Because it's cross-browser, I need to feature-detect for MutationObserver.  I do this by checking the existence of window.MutationObserver, window.WebKitMutationObserver, etc...


Actual results:

Now that MutationObserver returns null, that check fails, even though if I'm in Firefox in a jetpack Addon - I can create a new MutationObserver() successfully despite it returning null.

unfortunately this was a little bit of a tricky problem to get around, and rather than detecting the feature in Firefox, I must now (at least temporarily) simply assume that if your'e in a jetpack SDK addon, MutationObserver exists and can be used.


Expected results:

if I check for the existence of MutationObserver, it should not return null.
Moving to the SDK product, because I think this is failing with our proxy/sandbox implementation.
Component: Untriaged → General
Product: Firefox → Add-on SDK
Version: 19 Branch → unspecified

Comment 2

5 years ago
My research, which needs independent verification: https://gist.github.com/andytuba/5122272

I found two weird things.  One makes sense, the other does not.
My conclusion was that, if you `var MutationObserver` in the plugin's global scope, that makes a new MutationObserver in that scope.  Once you take out the var declaration, the plugin can access window.MutationObserver.    This ... makes sense, I guess.

Although window.MutationObserver normally with `typeof` checks, boolean statements, casting to string, and using it to instantiate a MutationObserver object, it still evaluates to null if you don't cast it or something first.  This is weird.
Alex, can you take a look at this?
Assignee: nobody → poirot.alex
Priority: -- → P2
This issue may be fixed by the removal of content script proxies.
Assignee: poirot.alex → nobody
This is still an issue in FF27
(Assignee)

Comment 6

5 years ago
Nothing is changed in FF29, the results are still the same published in https://gist.github.com/andytuba/5122272.

Notice that this is not seems only related to `MutationObserver`, but to *any* constructor I tried so far. For example, `Worker` and `CustomEvent` behaves in the same way:

    console.log(window.Worker, typeof window.Worker) // null, function
    console.log(window.CustomEvent, typeof window.CustomEvent) // null, function

Gabor, maybe you could have an hint about it?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(gkrizsanits)
(Assignee)

Updated

5 years ago
Summary: MutationObserver returns null in FF19+ in Addon SDK content scripts → Constructors (e.g. MutationObserver, Worker, CustomEvent…) returns null in Addon SDK content scripts
(Assignee)

Comment 7

5 years ago
Notice that the checks are not failing:

    console.log(window.MutationObserver);

    if(window.MutationObserver)
      console.log('MutationObserver is supported')
    else
      console.log('MutationObserver is not supported')

It's definitely true that the first `log` returns `null`, but the check passes, and log properly that "MutationObserver is supported".

So I'm wondering if this is just some `console.log` issues with `XrayWrapper`…?
This is super weird, I will look into this soon. I know for sure that the relationship between the sandbox of the content script, that has the content window hooked up in its proto chain, and the content window itself is somewhat weird... What I suspect the reason for the difference between console.log and the if check behavior might be that they are executed from different scope, and the Sandbox proxy changes behavior accordingly... It's really just a wild guess I have to read some code...
Flags: needinfo?(gkrizsanits)
Assignee: nobody → gkrizsanits
(Assignee)

Comment 9

5 years ago
As requested by gabor, a quick way to test is created such add-on:

    require('sdk/page-mod').PageMod({
      include: '*',
      contentScript: 'console.log(window.CustomEvent, typeof window.CustomEvent, window.CustomEvent ? "ok" : "no")'

And then open any page. I will log `null, function, ok'
i think this is actually much simpler than it seems, and has nothing to do with proxies, sandboxes, proto-chains, xray-wrappers or any other "magical" thing.

and despite the bug title, it isn't limited to Constructor functions, but applies to any method, or [object Function], and not just on the window object:

    console.log(document.addEventListener, typeof document.addEventListener)  // null, function

i'm positive it's caused by JSON serialization, used for message passing from content scripts to console, similar to bug 938326, and more than a dozen other bugs in the SDK.

we really need a better solution for console and error reporting in content scripts..  :(

can someone familiar with JSAPI check out what the DOM team is doing for Workers console in bug 620935 and see if it can be used for the SDK as well?
Flags: needinfo?(gkrizsanits)
Summary: Constructors (e.g. MutationObserver, Worker, CustomEvent…) returns null in Addon SDK content scripts → Constructors (e.g. MutationObserver, Worker, CustomEvent…) and method functions are logged as "null" in Addon SDK content scripts
(In reply to Tomislav Jovanovic [:zombie] from comment #10)
> i think this is actually much simpler than it seems, and has nothing to do
> with proxies, sandboxes, proto-chains, xray-wrappers or any other "magical"
> thing.

Agree. I've been playing with this for a while and had the same conclusion.
Also, it's a not too old regression.

> 
> and despite the bug title, it isn't limited to Constructor functions, but
> applies to any method, or [object Function], and not just on the window
> object:
> 
>     console.log(document.addEventListener, typeof document.addEventListener)
> // null, function
> 
> i'm positive it's caused by JSON serialization, used for message passing
> from content scripts to console, similar to bug 938326, and more than a
> dozen other bugs in the SDK.

Can be... I have no idea, I'm not that familiar with the SDK code, so the best if
I pass back the ball...

> 
> we really need a better solution for console and error reporting in content
> scripts..  :(
> 
> can someone familiar with JSAPI check out what the DOM team is doing for
> Workers console in bug 620935 and see if it can be used for the SDK as well?

Also, can someone tell me why was this working not that long time ago? (FF26)
Assignee: gkrizsanits → nobody
Flags: needinfo?(gkrizsanits)
(Assignee)

Comment 12

4 years ago
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #11)

> Also, can someone tell me why was this working not that long time ago? (FF26)

We switched to browser console, but I do not remember in which Firefox's version we land that, but was definitely around that version. Also, if it was earlier, it could be that the global / browser console get a regression – and we simple inherit it.

I will check with the rest of the devtools team during this work week.
(Assignee)

Updated

4 years ago
Summary: Constructors (e.g. MutationObserver, Worker, CustomEvent…) and method functions are logged as "null" in Addon SDK content scripts → Functions are logged as "null" in Addon SDK content scripts
(Assignee)

Updated

4 years ago
Duplicate of this bug: 973800
(Assignee)

Comment 14

4 years ago
Created attachment 8377750 [details] [review]
Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1405

It turned out to be a old `replacer` function used in `JSON.stringify`, that didn't propagate the function given as their source. It sounds odd to me that it worked properly prior FF26.
Attachment #8377750 - Flags: review?(jsantell)
(Assignee)

Updated

4 years ago
Assignee: nobody → zer0
Comment on attachment 8377750 [details] [review]
Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1405

Looks good, r+ once a test is added though
Attachment #8377750 - Flags: review?(jsantell) → review+

Comment 16

4 years ago
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/1ac5fbed27bb5213efd58480f4dc2876bc2066c4
Bug 849412 - Functions are logged as "null" in Addon SDK content scripts

- Updated the `replacer` for JSON.stringify in order to propagate functions as string only for `console` type of messages
- Added tests

https://github.com/mozilla/addon-sdk/commit/d38cdb2b81d48a72286357a7d8d6182425039035
Merge pull request #1405 from ZER0/function-null/849412

fix Bug 849412 - Functions are logged as "null" in Addon SDK content scripts r=@jsantell

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.