undefined document variable during initialisation of Sizzle when injected as content script of a Chrome extension

Description

I'm toying with an extension to chrome, and during this i have encountered issues when loading jQuery for use in content scripts. The content scripts execute in what chrome calls a 'isolated world', i.e., it may see all DOM object, but it is isolated from any scripts that was on the page they are injected into (and vice versa).

Currently i have isolated my problems down to a race condition, that seems(?) to arise due to the fact that my content script (currently only jQuery) is injected into the page before the DOM is fully initialised(?) or something like that.

What do you expect to happen?
The jQuery script being injected into the browsed page without any errors, such that it may be referenced from other content scripts.
What actually happens
I have tried to copy/paste and format the output i get from the chrome console. As noted before, this error only comes about 80% of the times. So if you try to reproduce, make sure to refresh the site a few times. It seems to always work on small pages like google.com, however on 'big' pages like newspapers, twitter, facebook, etc it seems to always fail with the below error:
- jQuery 1.12.4
```
jquery-1.12.4.js:940 Uncaught TypeError: Cannot read property 'createElement' of undefined
    assert                  @ jquery-1.12.4.js:940 
    (anonymous function)    @ jquery-1.12.4.js:2662
    (anonymous function)    @ jquery-1.12.4.js:2713
    (anonymous function)    @ jquery-1.12.4.js:34
    (anonymous function)    @ jquery-1.12.4.js:38
```
- jQuery 2.2.4
```
jquery-2.2.4.js:906 Uncaught TypeError: Cannot read property 'createElement' of undefined
    assert                  @ jquery-2.2.4.js:906
    (anonymous function)    @ jquery-2.2.4.js:2628
    (anonymous function)    @ jquery-2.2.4.js:2679
    (anonymous function)    @ jquery-2.2.4.js:34
    (anonymous function)    @ jquery-2.2.4.js:38
```
- jQuery 3.1.1
```
jquery-3.1.1.js:927 Uncaught TypeError: Cannot read property 'createElement' of undefined
    assert                  @ jquery-3.1.1.js:927
    (anonymous function)    @ jquery-3.1.1.js:2746
    (anonymous function)    @ jquery-3.1.1.js:2797
    (anonymous function)    @ jquery-3.1.1.js:36
    (anonymous function)    @ jquery-3.1.1.js:40
```
I did some debugging in the 2.2.4 version

From the above errors, it is kinda clear (and i debugged it), that document is somehow (sometimes) ending up as undefined.
```
905: function assert( fn ) {
906:    var div = document.createElement("div");
907:
```
I boiled it down to the internals of Sizzle, where it should set the internal document variable, which is used in the above assert function. However, this setDocument() function sometimes returns early if "doc is invalid or already selected"
```
2623: // Initialize against the default document
2624: setDocument();
```
By printing the document variable before and after, i noticed that it is sometimes still undefined after this function, which leads to the above mentioned uncaught type errors.

From what i can debug, it all comes down to this
```
1042:   // Return early if doc is invalid or already selected
1043:   if ( doc === document || doc.nodeType !== 9 || !doc.documentElement ) {
1044:       return document;
1045:   }
```
Printing doc, document, doc.nodeType and doc.documentElement to the console, always yielded in
- doc being a HTMLDocument object,
- document being undefined, as it hasn't been initialised yet (we are currently doing it),
- doc.nodeType equal to 9,
  but
- doc.documentElement was either a HTMLHtmlElement (then obviously it worked), or null and then obviously it didn't work, as it would 'return early' with document still being undefined.
Which browsers are affected?

Currently I have tested this on a Debian machine (Linux evelin 4.6.0-1-amd64 Accumulated, not yet pulled commits #1 SMP Debian 4.6.4-1 (2016-07-18) x86_64 GNU/Linux) with Google Chrome (stable 53.0.2785.116-1). As this is done using a Chrome extension to inject jQuery into the running page i don't really have a means to reproduce this in any other browser.

I don't know if this is a Chrome bug or jQuery, as it seems timing related and that I don't know enough about javascript to say for sure what can be 100% expected to be initialized by the browser before any script is being run. However currently i have fixed it by placing some busy looping code in the top of the jQuery script, to make sure it is delayed just a few fractions of a sec, that it loads without any errors.

Link to test case

You need a empty directory. In here you place this "manifest.json" file:

{
  "manifest_version": 2,
  "name": "jQuery MWE",
  "version": "0.1",

  "background": {
    "persistent": false,
    "scripts": [
      "eventPage.js"
    ]
  },

  "permissions": [
    "declarativeContent",
    "<all_urls>"
  ]
}

and you then place this "eventPage.js" file along side (which is responsible for injecting the jQuery file as a content script into the maching web page, which here is any page you browse).

var show_page_action = {
    conditions: [
        // https://developer.chrome.com/extensions/declarativeContent#type-PageStateMatcher
        new chrome.declarativeContent.PageStateMatcher({
            pageUrl: {
                urlMatches: '.*',
                //schemes: ['https'],
            }
        })
    ],
    actions: [
        new chrome.declarativeContent.RequestContentScript({
            js: ["jquery-2.2.4.js",],
        })
    ],
};

// Make sure our declarativeContent rules are up to date.
chrome.runtime.onInstalled.addListener(function (details) {
    chrome.declarativeContent.onPageChanged.removeRules(undefined, function() {
        chrome.declarativeContent.onPageChanged.addRules([show_page_action])
    })
});

Then of cause you need the jQuery file (above i have used "jquery-2.2.4.js") and then you just need to load the directory as a chrome extension.

When the extension is loaded, go browse any webpage with the chrome developer tools/console open, and refresh the page a few times until you see the above mention uncaught type error. Most of the times google.com will work just fine as it seems to load fast enough, so try something with a bit more content on.

Thank you for opening an issue! However, your test case requires a lot of work on our end just to get to the point where we are debugging the issue. I think this needs to be fleshed out more before we can continue. It's very odd that document is undefined. That should be available even before the DOM is parsed. In other words, if you find out more about the issue and can confirm that there is something we can change in jQuery to fix it, we can reopen this ticket.

@timmywil

@timmywil, Sure, you are right. It requires more than clicking a jsfiddle link, as you are probably used to.
But what do you want/expect from me, that I haven't already supplied you with?

To be honest, I find i quite rude to just close the ticket, before giving me a chance to reply to your comment and/or before concluding that nothing new will be brought to the table.

I could perhaps check the files into a repository, but checking that out or copy/pasting the two snippets into their own files is almost equally easy for you or anyone else to do.
Putting the files into a directory, and loading the directory in Chrome as an extension can be done in 1-2 minutes.

My knowledge of jQuery internals is quite limited. I could come up with quite a few things, but I would have no idea what consequences they would have.
I assume there must be a reason why all that elaborate testing is done in the setDocument function.
And a reason why the internal document variable isn't just initialised to be equal to preferredDoc, since this seems to be the desired side effect of calling setDocument without a node argument` on line 2624.

In all circumstances, I would say that it at least deserves a proper error response, instead of returning an undefined document variable, which throws an exception further down the execution stack, as it assumes everything went fine on line 1044. In fact it seems that the document variable should never be able to be undefined.

However even if one somehow changed the code to not erroneously "return early" in the case where we are trying to initialise against the default document. Then, as far as I can see, the undefined document.documentElement is saved in the local variable docElem just below, which is used further down.
So i guess the only proper solution would be to somehow wait for the document.documentElement to come into existence? I guess it is bound to get around at some point, as even an empty page always seems to get some minimal html with an empty head and empty body.
The case where one would have to wait, for the document.documentElement to be populated by the browser, would not hurt anyone, as in theory there is "nothing" else in existence.

@reenberg

@reenberg I had no intention of being rude. Maybe there really is nothing more you can provide, but closing a ticket does not mean it can't be reopened. It is a tool for triaging to narrow the tickets we can focus on and get fixed.

setDocument in Sizzle has been used for a while, and I don't see anything obviously wrong with it. If the document variable is undefined, I would normally say we don't support environments that don't have a window with a document. Sizzle is only the first place that would throw an error when there's not a document, so many more changes would be required to work the way you're describing. And we can't make those changes without pinpointing the exact problem.

I hope that's a better explanation. Thank you again for contributing. Even if we don't pinpoint the problem immediately, having this issue here for posterity is worthwhile.

You have to differentiate between which document variable there is in play.

Inside Sizzel (is that the right term to use?), there is defined a local document variable at line 572, which (as I understand from the code) is updated through the setDocument function so that sizzle may work in different contexts.
The global document variable (window.document) is defined, and it is even saved as preferredDoc at line 582.

The issue at hand is that the global document.documentElement (window.document.documentElement) is sometimes not present when the script is run.
This makes sense if Chrome injects the content scripts before the DOM has been initialized(again not sure if I'm using the right terms?). I believe this is the case as I can see (if I drop to the debugger at this stage) that the browser window/tab is all empty.

I would argue that I have already pinpointed the exact issue: setDocument() is called when window.document.documentElement is undefined, resulting in it not initialising the local document variable because it "returns early", resulting in an uncaught TypeError later, as the code tries to do a document.createElement("div"); inside the assert function on the undefined local document variable.

What can be argued is whether my reasoning for it happening is correct, and of cause whether this is a bug in Chrome or in jQuery.
I don't know which guarantees a script may expect, in regards to its execution environment.

If one are always to expect that there is a global document.documentElement when a script is being executed, then this is clearly a Chrome bug. But if this is not defined anywhere, then i would say that jQuery should handle this.

No matter which of the above, I would still say that jQuery should fail more gracefully if the internal document is undefined. That would help in the future if anyone is ending up in the same kind of state as I.
I found #15014 in your old bugtracker(?) and i believe that he is actually experiencing the exact same problem as me: The internal document variable is undefined, most likely because of the same reason as im experiencing it (though i don't know anything about the ChromeEmbeddedFramework). Note that i don't agree in his solution at all (it is in fact wrong as far as i can see) or in any of his reasoning. I just linked it to underline that I'm most likely not the only one who has ended up with this issue, and thus jQuery ought to at least fail gracefully telling me that document.documentElement is undefined.

Perhaps it would make sense to do such a test, after the call to setDocument() on line 2625, as at that point jQuery/Sizzle will fall apart if execution is continued on an undefined local document variable.
And as this is an initialisation, that should only be run once(?) and thus not a performance hit to anyone.

due to the fact that my content script (currently only jQuery) is injected into the page before the DOM is fully initialised(?) or something like that.

This isn't very firm information to use when asking for a change. In all the environments we formally support there is a document. Anecdotally I know that others are using jQuery in Chrome extensions, I have never used it there myself. Perhaps you can research this and find out why their uses succeed and this one fails?

@dmethvin

Perhaps you can research this and find out why their uses succeed and this one fails?

I also know that many other people are using jQuery in their Chrome extensions. But I'm quite sure it is because they are not using the chrome.declarativeContent.RequestContentScript to inject the content script, but the old way of injecting by either specifying it in the manifest.json file or by programmatic injection through chrome.tabs.executeScript in the backend script/event handler.

The RequestContentScript is quite new, and in the documentation it is marked as not being available in the stable release of chrome. Though my tests showed that it actually is available in the stable branch (as starting of this year or somewhere around that). As far as i can get from their internal bug tracker, they haven't removed those remarks from the API documentation since the API haven't been officially vetted or something, and as such they may decide to change it in the future once it actually gets looked through. If that is ever going to happen (their own bug reports on the matter dates 1-2 years back).

Again, it is technically not the document that is undefined, it is the document.documentElement, that has not yet been set.

I guess it is fair enough to say that you only formally support environments where this is not undefined at the moment where jQuery is loaded. However I'm still missing a hard evidence that says that document.documentElement must exists when javascripts are executed, before I can go and report it as a bug to Chrome, else it would just be a feature request (I assume).

Since you do have the option of injection the content scripts at various stages of the page loading, if done by the manifest.json file via the run_at specifyer:

In the case of "document_start", the files are injected after any files from css, but before any other DOM is constructed or any other script is run.

In the case of "document_end", the files are injected immediately after the DOM is complete, but before subresources like images and frames have loaded.

In any case, @dmethvin, you don't address my point whether or not jQuery should fail more gracefully in this case where it is run in a non formally supported environment?

@dmethvin

In any case, @dmethvin, you don't address my point whether or not jQuery should fail more gracefully in this case where it is run in a non formally supported environment?

I can address it by saying that it is very difficult to anticipate all the things that might go wrong when the code is used in untested environments. Doing it in the dark as a thought experiment isn't going to work that well. As said elsewhere, once you "solve" this by trying to get past the lack of document here, it's likely you'll run into the same problem somewhere else in Sizzle or jQuery.

I'm not suggesting to try to solve issue of a missing document. By failing gracefully i mean throwing an exception, with a proper error message describing the lack of a document.documentElement, instead of just letting it keep executing in a bad state, just to have it throw an exception elsewhere that makes no sense as to what the original problem was.

There are all sorts of assumptions the code makes about the state of the environment it is running within. Those assumptions are met by the supported environments. It's a waste of code to put in a bunch of checks and error messages for those when they are never supposed to happen. The reason the exception makes no sense is because the state of the environment makes no sense to the code. A clear error message won't help you here, it still won't make the code run.

document is not set in some places - even if it's defined on the top. it should be "window.document". it's not working in electron neither for me.

@reenberg

I have the same issue with a similar use-case as @reenberg ... Did you solve your issue with a specific workaround?

Navigation Menu

Repository navigation

undefined `document` variable during initialisation of Sizzle when injected as content script of a Chrome extension #3333

Description

Link to test case

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

undefined document variable during initialisation of Sizzle when injected as content script of a Chrome extension #3333

Description

Description

Link to test case

Activity

timmywil commented on Sep 26, 2016

reenberg commented on Sep 26, 2016

timmywil commented on Sep 26, 2016

reenberg commented on Sep 27, 2016

dmethvin commented on Sep 27, 2016

reenberg commented on Sep 27, 2016

dmethvin commented on Sep 27, 2016

reenberg commented on Sep 27, 2016

dmethvin commented on Sep 27, 2016

vandalouze commented on Apr 6, 2017

GeoloeG-IsT commented on Jun 29, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

undefined `document` variable during initialisation of Sizzle when injected as content script of a Chrome extension #3333