538156 - sf.eater.com and sf.curbed.com goes blank after page loads. Likely due to facebook script.

Status: RESOLVED FIXED

(Tech Evangelism Graveyard :: English US, defect)

Description

[Mike Beltzner [:beltzner, not reading bugmail]]

http://sf.eater.com
http://sf.curbed.com

The page loads, then as soon as it seems complete (after waiting for ad scripts from another server) for some reason the document location is changed and Firefox reloads. This second load never completes, and no graphics or text is shown in the window.

From the user's perspective, the page loads then unloads and hangs. It does not do this in any other browser as far as I can tell.

Started happening at least in Beta 5 according to Melissa (who reported it to me) and the behaviour makes me suspect asynchronous page loading, but we need to figure out what's going on.

Comment 1

[Melissa Shapiro [:shappy]]

Confirming that this started in Beta 5.  I'm a regular reader of these blogs and never had any problems loading the pages until recently.

Comment 2

[Boris Zbarsky [:bzbarsky]]

OK, so what happens here is that document.write is called on the main page after parsing is done, which of course blows away the page.

The C++ stack to the call:

#1  0x12db266e in nsHTMLDocument::Write (this=0x1b373e00, aText=@0xbfffb9e4) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/html/document/src/nsHTMLDocument.cpp:2203
#2  0x00c450e8 in nsIDOMHTMLDocument_Write (cx=0x138b400, argc=1, vp=0x129bb98) at dom_quickstubs.cpp:12769
#3  0x0031d4fc in js_Interpret (cx=0x138b400) at jsops.cpp:2263
#4  0x0033143a in js_Invoke (cx=0x138b400, argc=2, vp=0x129baf8, flags=0) at jsinterp.cpp:1384
#5  0x002f603d in js_fun_apply (cx=0x138b400, argc=2, vp=0x129bab0) at /Users/bzbarsky/mozilla/vanilla/mozilla/js/src/jsfun.cpp:2041
#6  0x0031d4fc in js_Interpret (cx=0x138b400) at jsops.cpp:2263
#7  0x00330661 in js_Execute (cx=0x138b400, chain=0x16106e40, script=0x1b600200, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1618
#8  0x0029ebf1 in JS_EvaluateUCScriptForPrincipals (cx=0x138b400, obj=0x16106e40, principals=0x1ed42e24, chars=0x15fde008, length=18683, filename=0x1ed12e78 "http://static.ak.connect.facebook.com/js/api_lib/v0.4/FeatureLoader.js.php/en_US", lineno=1, rval=0x0) at /Users/bzbarsky/mozilla/vanilla/mozilla/js/src/jsapi.cpp:5064
#9  0x12ecbcad in nsJSContext::EvaluateString (this=0x1f175420, aScript=@0x1ed12ae4, aScopeObject=0x16106e40, aPrincipal=0x1ed42e20, aURL=0x1ed12e78 "http://static.ak.connect.facebook.com/js/api_lib/v0.4/FeatureLoader.js.php/en_US", aLineNo=1, aVersion=0, aRetValue=0x0, aIsUndefined=0xbfffca94) at /Users/bzbarsky/mozilla/vanilla/mozilla/dom/base/nsJSEnvironment.cpp:1752
#10 0x12c711be in nsScriptLoader::EvaluateScript (this=0x1ed51c70, aRequest=0x1ed12ad0, aScript=@0x1ed12ae4) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsScriptLoader.cpp:713
#11 0x12c7147a in nsScriptLoader::ProcessRequest (this=0x1ed51c70, aRequest=0x1ed12ad0) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsScriptLoader.cpp:626
#12 0x12c7165c in nsScriptLoader::ProcessPendingRequests (this=0x1ed51c70) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsScriptLoader.cpp:786
#13 0x12c71956 in nsScriptLoader::OnStreamComplete (this=0x1ed51c70, aLoader=0x1c6c5260, aContext=0x1a5b1bf0, aStatus=0, aStringLen=66, aString=0x166980c0 "__jsonpretweets14({\"clicks\":38,\"shorturl\":\"http://bit.ly/8JDLt9\"})") at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsScriptLoader.cpp:974

The JS stack:

0 anonymous() ["http://static.ak.connect.facebook.com/js/api_lib/v0.4/FeatureLoader.js.php/en_US":17]
    a = undefined
    this = [object Object]
1 anonymous(a = undefined, b = [object Object], c = "FB.HiddenContainer") ["http://static.ak.connect.facebook.com/js/api_lib/v0.4/FeatureLoader.js.php/en_US":13]
    d = [object Object]
    this = [object Object]
2 anonymous("FB.HiddenContainer", [object Object]) ["http://static.ak.connect.facebook.com/js/api_lib/v0.4/FeatureLoader.js.php/en_US":13]
    e = undefined
    this = [object Object]
3 <TOP LEVEL> ["http://static.ak.connect.facebook.com/js/api_lib/v0.4/FeatureLoader.js.php/en_US":17]
    this = [object Window @ 0x1f176af0 (native @ 0x1f175b90)]

Comment 3

[Boris Zbarsky [:bzbarsky]]

There is one async script on the page, and that's http://www.google-analytics.com/ga.js

Comment 4

[Boris Zbarsky [:bzbarsky]]

The FeatureLoader thing is obfuscated up the wazoo (and even confuses jsbeautifier into going into what looks like close to an infinite loop), sadly.

In any case, the behavior first appears on m-c between the 2009-01-13 build (rev 9dbded90af2a) and the 2009-01-14 build (rev 89811ac1b35a).  Pushlog range:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9dbded90af2a&tochange=89811ac1b35a

Nothing in there obviosly jumps out at me.  Will try to bisect down to the relevant changeset.

Comment 5

[Boris Zbarsky [:bzbarsky]]

And naturally I can't actually compile builds from back then on my fast box, because of the pango idiocy.  :(

I'll try to get that done on my laptop, but it'll take a while.

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

I suggested that the culprit might be bug 347174 based on the regression range in comment 4, and bz said he'd start in on that one. cc'ing people involved in that bug.

If someone can beat bz to a local backout of that change to determine if it's the culprit, by all means let us know!

Comment 7

[Jim Jeffery not reading bug-mail 1/2/11]

FWIW, the pages do the same thing on today's m-c trunk using Win7 64bit HP

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100106 Minefield/3.7a1pre Firefox/3.6 ID:20100106050126

Comment 8

[Boris Zbarsky [:bzbarsky]]

I looked into when we start the load for the script that's running in the JS stack in comment 2.  It happens when that <script> element is inserted into the DOM, which is done at this JS stack:

0 anonymous([object Event @ 0x7fffdaf520f0 (native @ 0x7fffdaf48140)]) ["http://cdn0.cstatic.net/javascripts/prototype.js,getjson.js,effects.js,cufon.js,accordion.js,carousel.js,livepipe.js,tabs.js,swfobject.js,network.js,comments/comments.js?2522":12]
    b = false
    a = [object HTMLScriptElement @ 0x7fffdafb1160 (native @ 0x7fffdaf9ef20)]
    this = [object HTMLDocument @ 0x7fffde2bb040 (native @ 0x7fffde28c000)]
1 anonymous(p = [object Event @ 0x7fffdaf520f0 (native @ 0x7fffdaf48140)]) ["http://cdn0.cstatic.net/javascripts/prototype.js,getjson.js,effects.js,cufon.js,accordion.js,carousel.js,livepipe.js,tabs.js,swfobject.js,network.js,comments/comments.js?2522":8]
    this = [object HTMLDocument @ 0x7fffde2bb040 (native @ 0x7fffde28c000)]
2 anonymous(j = undefined, k = "dom:loaded", l = [object HTMLDocument @ 0x7fffde2bb040 (native @ 0x7fffde28c000)]) ["http://cdn0.cstatic.net/javascripts/prototype.js,getjson.js,effects.js,cufon.js,accordion.js,carousel.js,livepipe.js,tabs.js,swfobject.js,network.js,comments/comments.js?2522":8]
    m = [object Event @ 0x7fffdaf520f0 (native @ 0x7fffdaf48140)]
    this = [object Window @ 0x7fffe5b29710 (native @ 0x7fffe5b92c58)]
3 anonymous("dom:loaded") ["http://cdn0.cstatic.net/javascripts/prototype.js,getjson.js,effects.js,cufon.js,accordion.js,carousel.js,livepipe.js,tabs.js,swfobject.js,network.js,comments/comments.js?2522":1]
    m = [object HTMLDocument @ 0x7fffde2bb040 (native @ 0x7fffde28c000)],dom:loaded
    this = [object HTMLDocument @ 0x7fffde2bb040 (native @ 0x7fffde28c000)]
4 a([object Event @ 0x7fffde2b1f90 (native @ 0x7fffdb1cfb20)]) ["http://cdn0.cstatic.net/javascripts/prototype.js,getjson.js,effects.js,cufon.js,accordion.js,carousel.js,livepipe.js,tabs.js,swfobject.js,network.js,comments/comments.js?2522":1]
    this = [object HTMLDocument @ 0x7fffde2bb040 (native @ 0x7fffde28c000)]

Looking at the C++ stack, this is a handler for a "dataavailable" event dispatched from script via dispatchEvent; that dispatchEvent call is done from inside a DOMContentLoaded handler.  So in fact, that script is guaranteed to run at a point at which a document.write will blow away the document.

Still working on that local backout....

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

It would be extremely unfortunate if the google analytics script is the culprit here. They've definitely tested with firefox builds that support async while they developed the current version of the analytics script.

So it sounds like all signs are pointing at the document.readystate thing. That didn't change in beta 5 though, but much much earlier. Maybe the site changed recently?

Melissa, could you possibly test with an earlier beta? Any of them should do. And see if this now happens there too.

Comment 10

[Boris Zbarsky [:bzbarsky]]

OK, local backout gives me this range so far:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=03bc3379822a&tochange=89811ac1b35a

So certainly looks like the readyState thing.

Comment 11

[Melissa Shapiro [:shappy]]

@jonas - i only run a single instance of firefox on my machine.  but the issue afaict isn't specific to my machine/profile.  if someone on this bug has b4 or earlier, perhaps they can try to load the page and report back?

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

For some context:

document.readyState is an IE feature that we have implemented. It's also in HTML5. We've run into trouble with this property before. It appears that it's heavily used on the web, and so implementing the property affects many sites. Some of which are bound to react poorly to the fact that the feature now behaves differently in firefox (exists, whereas it didn't used to).

Given that, I think this is unfortunately an evangelism bug.

Will test in earlier betas to make sure before moving over to evangelism.

Comment 13

[Boris Zbarsky [:bzbarsky]]

OK.  The build without the readyState implementation still starts the load of the http://static.ak.connect.facebook.com/js/api_lib/v0.4/FeatureLoader.js.php/en_US script from DOMContentLoaded.  However there are no nsHTMLDocument::WriteCommon calls after that point.  So the real question now is why that script calls write() in the build with readyState support...

Line 17 of the obfuscated script, which is the one that calls write(), looks like this after the jsbeautifier treatment:

FB.provide('FB.HiddenContainer', {
    _onLoad: function () {
        if (document.getElementById('FB_HiddenContainer') == null) {
            var a;
            try {
                if ((!document.readyState || document.readyState == "complete") && document.body) {
                    a = document.createElement('div');
                    a.id = "FB_HiddenContainer";
                    a.style.position = "absolute";
                    a.style.top = "-10000px";
                    a.style.width = "0px";
                    a.style.height = "0px";
                    document.body.appendChild(a);
                }
            } catch(e) {
                a = null;
            }
            if (!a) document.write('<div id="FB_HiddenContainer" ' + 'style="position:absolute; top:-10000px; left:-10000px; width:0px; height:0px;" >' + '</div>');
        }
    },
    get: function () {
        return FB.$('FB_HiddenContainer');
    }
});

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

Boris already found a regression range that predates Beta 5, so I'm pretty sure that this is a recent change to the *.eater.com and *.curbed.com network which triggered the document.readyState

My fear, however, is that it's some commonly used script served to Firefox-but-not-others which is causing the problem.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Hmm.  So if that function runs during onload, which is what it seems like it should do, then we should have readyState == "complete", in fact.

But given the stack in comment 2, this function is NOT in fact being called from the onload handler for some reason, hence the trouble.

Comment 16

[Boris Zbarsky [:bzbarsky]]

I suppose we should try to figure out line 13; it looks like the call to provide() above ends up calling some other function on line 13, which turns around and calls into the _onLoad here...

Comment 17

[Boris Zbarsky [:bzbarsky]]

Line 13 is:

if (!window.FB) FB = {};
FB.forEach = function (c, a, f) {
    if (Object.prototype.toString.apply(c) === '[object Array]') {
        if (c.forEach) {
            c.forEach(a);
        } else for (var b = 0, e = c.length; b < e; b++) a(c[b], b, c);
    } else for (var d in c) if (f || c.hasOwnProperty(d)) a(c[d], d, c);
};
FB.copy = function (c, b, a) {
    FB.forEach(b, function (e, d) {
        if (a || typeof c[d] === 'undefined') c[d] = e;
    });
};
FB.copy(FB, {
    $: function (a) {
        return document.getElementById(a);
    },
    TypeLoader: {
        NOTIFY: {},
        LOADED_MODULES: {},
        LOADED_CLASSES: {},
        resolve: function (f, e, a) {
            for (var b = 0, c = e.length; b < c; b++) {
                var d = e[b];
                if (d === '') continue;
                if (typeof f[d] === 'undefined') if (a) {
                    f[d] = {};
                } else return false;
                f = f[d];
            }
            return f;
        },
        provide: function (c, b, a) {
            FB.Monitor.wrapObject(b, c, false);
            var d = this.resolve(window, c.split('.'), true);
            if (a || !this.LOADED_MODULES[c]) {
                FB.copy(d, b, true);
                if (b._onLoad) d._onLoad();
            }
            this.LOADED_MODULES[c] = true;
        },
        subclass: function (e, a, i) {
            if (this.LOADED_CLASSES[e]) return;
            if (a !== 'FB.Class' && !this.LOADED_CLASSES[a]) {
                FB.Log.debug('"' + e + '" needs to wait for "' + a + '"');
                var j = FB.redo(arguments, this);
                this.NOTIFY[a] ? this.NOTIFY[a].push(j) : this.NOTIFY[a] = [j];
                return;
            }
            var b = this.resolve(window, a.split('.')),
                f = b.extend(i),
                g = this.NOTIFY[e],
                h = e.split('.'),
                d = this.resolve(window, h, true),
                c = h.pop();
            FB.Monitor.wrapObject(f.prototype, e, true);
            FB.Monitor.wrapObject(f, e, false);
            FB.copy(f, d);
            this.resolve(window, h)[c] = f;
            this.LOADED_CLASSES[e] = true;
            if (g) {
                FB.forEach(g, function (k) {
                    k();
                });
                delete this.NOTIFY[e];
            }
        }
    },
    bind: function () {
        var a = Array.prototype.slice.call(arguments),
            c = a.shift(),
            b = a.shift();
        var d = function () {
            ++FB.Monitor.internalCallStackDepth;
            var e = c.apply(b, a.concat(Array.prototype.slice.call(arguments)));
            --FB.Monitor.internalCallStackDepth;
            return e;
        };
        d._targets = [b, c];
        return d;
    },
    redo: function (a, c) {
        var b = Array.prototype.slice.call(a);
        b.unshift(c);
        b.unshift(a.callee);
        return FB.bind.apply(FB, b);
    }
});
FB.provide = FB.bind(FB.TypeLoader.provide, FB.TypeLoader);
FB.subclass = FB.bind(FB.TypeLoader.subclass, FB.TypeLoader);

Comment 18

[Boris Zbarsky [:bzbarsky]]

OK.  So FB.provide is a function that takes some arguments, then calls calls FB.TypeLoader.provide with |this| set to FB.TypeLoader and the given arguments.

In particular, in our case it'll call FB.TypeLoader.provide with c == 'FB.HiddenContainer', b == that big object, and a == the get function from comment 13, right?

So in FB.TypeLoader.provide |a| is a function, so tests true and we call the _onLoad method right then.

There's no guarantee that any of this will happen before onload fires, that I can see; in particular no guarantee that document.readyState == "complete".  At least in Gecko.  Is it possible that IE/Safari are delaying execution of this script to after onload for some reason?  Or that they're getting non-null for document.getElementById('FB_HiddenContainer') for some reason?

Comment 19

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Ok, created two testcases:

http://people.mozilla.org/~sicking/tests/docreadystate/docreadystateIE.html
http://people.mozilla.org/~sicking/tests/docreadystate/docreadystate.html

IE is painful enough to deal with that I just created a separate testcase. In particular they don't support addEventListener :(

So as far as readyState goes, I think we are behaving correctly. We largely behave like webkit/safari/chrome. The only difference is that during DOMContentLoaded they have readyState == "loaded" (*not* "loading"), whereas we have readyState == "interactive".

Our behavior matches IE and the HTML5 spec. I can't get IE to have readyState == "loaded" at all, but it does have readyState == "interactive" while running deferred scripts.

Comment 20

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

In particular, the test in the code in comment 13

if ((!document.readyState || document.readyState == "complete")
    && document.body) {
  ...
}

should behave the same in gecko and webkit.

But, as you say in comment 18, there's a lot of other things that can be different here.

I think what we should do is to evangelize facebook to change the test above to be:

if (!document.readyState || document.readyState != "loading") {
  ...
}

You really don't want to call document.write otherwise.

Comment 21

[Boris Zbarsky [:bzbarsky]]

I'm happy with evangelizing the facebook folks, but I'm going to take a bit and see if I can answer my questions from comment 18 with some nsScriptLoader hackery to redirect that load....

Comment 22

[Mike Beltzner [:beltzner, not reading bugmail]]

Do we know if this is a FB script (like, where is it hosted?) or a script on the eater.com network meant to interact with Facebook? I'm not having problems on other sites with FB connect, for example, like Lifehacker.

Comment 23

[Boris Zbarsky [:bzbarsky]]

> like, where is it hosted?

http://static.ak.connect.facebook.com/js/api_lib/v0.4/FeatureLoader.js.php/en_US like comment 2 says.  ;)

Comment 24

[Boris Zbarsky [:bzbarsky]]

Oh, and it looks like the comments.js file is what loads that file.

I tried creating a local version of this page, and it doesn't show the bug; presumably either timing is somewhat different or just some of the dynamic loading it does didn't get fixed up enough....

Comment 25

[Mike Beltzner [:beltzner, not reading bugmail]]

OK, reached out to both Facebook (hi, Wei!) and eater/curbed.com

Facebook says the code is their bootloader, but it's curious to me as to why it's only causing problems on the eater/curbed network so far (I haven't seen it elsewhere, like Lifehacker, which uses FB connect). Can anyone find this happening on other sites that use the "people who've commented about this" feature that I believe eater/curbed is using?

Is it possible that it's due to the way it's referenced in the eater/curbed scripts?

Comment 26

[Boris Zbarsky [:bzbarsky]]

Well, a key issue is the way the script is being loaded, yes.  It's being loaded after document parsing is done, from some CDN junk.  See comment 8.

What I'd really like out of facebook here is some idea why loading it that way doesn't cause the script to break in IE or Webkit.  From everything I can see, this script should be breaking in all browsers that implement readyState when loaded the way this site loads it, modulo the two possibilities I mention in comment 18.

Comment 27

[Wei Zhu]

Hi guys,
This Wei Zhu from Facebook. I just want to let you know what I will be investigating this bug from our side. How urgent is it? Would it be OK if I investigate on Monday? 

Wei

Comment 28

[Mike Beltzner [:beltzner, not reading bugmail]]

Hi Wei. We expect that Firefox 3.6 will be shipping in the next 2 weeks or so, but we will be publishing the release candidate tomorrow. The sooner we can get a confirmed fix for this, the better it will be for the 25% of the Internet who use Firefox! :)

(Monday should be fine.)

Comment 29

[Wei Zhu]

Hi Mike, I will look at it on Monday, then. I am pretty sure this is just something I need to change as my current code assumes that browser is not Firefox when document.readyState exists.

Comment 30

[Boris Zbarsky [:bzbarsky]]

I'd really love to know where it makes that assumption....  In the code above, all browsers should be behaving the same way.

Comment 31

[Wei Zhu]

Update: I've updated JS code for Facebook Connect to to avoid using document.write.

@Broris, I am making assumption that !document.readyState means "not firefox" in the code below. That was not a good enough.
try {
   if ((!document.readyState || document.readyState == "complete")
&& document.body) {
                    a = document.createElement('div');
                    a.id = "FB_HiddenContainer";
                    a.style.position = "absolute";
                    a.style.top = "-10000px";
                    a.style.width = "0px";
                    a.style.height = "0px";
                    document.body.appendChild(a);
                }
            } catch(e) {
                a = null;
            }

Comment 32

[Boris Zbarsky [:bzbarsky]]

Wei Zhu, where does that assume anything about Firefox?  In Safari, at the point this code executes, document.readyState should test true and not be set to "complete", and document.body should exist.  Same for IE.  But the document.write wasn't causing problems in those browsers...

Comment 33

[Wei Zhu]

I was just trying to avoid body.appendChild when the browser is IE and document.readyState != 'completed' because IE would have "operation aborted" error. body.appendChild is always fine on non-IE browsers such as Firefox and Safari and my preferred approach. document.write normally work fine for Firefox, but it does not seem to work well for Firefox 3.6.

This my new code now.

try {
  var isIE = window.navigator.userAgent.toLowerCase().indexOf('msie') >= 0
       && window.attachEvent;
  if ((!isIE || document.readyState == 'completed') && document.body) {
    hiddenDiv = document.createElement('div');
    ...
    document.body.appendChild(hiddenDiv);
  }
} catch (e) {
hiddenDiv = null;
}
if (!hiddenDiv) {
 document.write('<div id="FB_HiddenContainer" '
+ 'style="position:absolute; top:-10000px;
left:-10000px; width:0px; height:0px;" >'
+ '</div>');
}

Comment 34

[Boris Zbarsky [:bzbarsky]]

> document.write normally work fine for Firefox, but it does not seem to work
> well for Firefox 3.6

document.write at that point NEVER works for Firefox, for ANY version.  It's just that document.readyState tests false in Firefox < 3.6.

Your new code is fine, but I still don't understand how the old code could possibly have worked in any browser at all, as long as that browser implements document.readyState, and in particular why it didn't break in either Safari or IE (both of which implement document.readyState).  Is this code being entered at all on this page for those other browsers?

Comment 35

[Mike Beltzner [:beltzner, not reading bugmail]]

--> RESOLVED FIXED, thanks, Wei!