Closed Bug 453825 Opened 17 years ago Closed 16 years ago

window.top gets undefined on some pages if NoScript is installed but AdBlock Plus is not

Categories

(Core :: DOM: Core & HTML, defect, P3)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: ma1, Assigned: jorendorff)

References

(
URL
)

Details

(Keywords: fixed1.9.1, Whiteboard: \)

On http://mlb.mlb.com/index.jsp and http://www.zellers.com/stores/shop/home/en/zellers when NoScript is installed, even if scripts are globally allowed and the "Plugins|Block every object coming from a site marked as untrusted" preference (meaning that no CAPS policy is enforced and that installed content policy is NOP), window.top gets undefined during page load. The two pages (the only ones reported so far) have in common Flash content checking for its location by opening "javascript:top.location+"__flashplugin_unique__", and actually that's how the issue has been detected: user reporting the problem complained about the Flash content being "partially shown", and the error console repeatedly shows: <error_console> Error: top is undefined Source File: javascript:top.location+"__flashplugin_unique__" Line: 1 </error_console> Trying to execute "javascript:alert(window.top)" from the location bar shows that the property is undefined for web content as well. Interestingly, as soon as AdBlock Plus is installed (together with NoScript) *and enabled on that page*, the problem goes away, even if NoScript is reset to its default "closed down" configuration with complex content policy and enforced CAPS. Another interesting work-around for this issue with AdBlock Plus not installed is setting the following preferences: noscript.jsHack -> window.top = window.top noscript.jsHackRegExp -> http://(?:www\.zellers|mlb\.mlb)\.com/ jsHack is executed in the content document as a javascript: URL via location.href during onLocationChange for pages matching jsHackRegExp (none by default). In this case, the apparent NOP "window.top = window.top" suffices for the property to be "remembered". This let me suspect the property disappears during page load due to some refcounting bug. No idea of what AB+ does as a workaround, though, since nothing is shown as blocked.
I also see this on a little-known video website called youtube -- I get a blank white flash plugin in place of video and its controls, along with the "top is undefined" at javascript:top.location+"__flashplugin_unique__" nag. The breaking change is changeset: 18543:c7ce6c7f2d76 user: Jason Orendorff <jorendorff@mozilla.com> date: Tue Aug 19 21:38:24 2008 -0500 summary: Bug 407216 - DOM quick stubs - faster paths for top N DOM methods (r+sr=jst, security r=mrbkap, build r=bsmedberg)
Blocks: 407216
OS: Windows XP → All
Hardware: PC → All
Smaller test case: <!DOCTYPE HTML> <body> <div id="x"></div> <script type="text/javascript"> var n=document.getElementById("x"); alert("before: " + top); n.innerHTML="<embed type=\"application/x-shockwave-flash\" src=\"missing.swf\"/>"; alert("after: " + top); </script> </body>
This could probably be fixed by not putting a quick stub on window.top. Comment out the appropriate line of js/src/xpconnect/src/dom_quickstubs.qsconf . I'd sure like to know exactly what the interaction is between NoScript and Flash that's causing this, though. (Incidentally, window.top=window.top is not a no-op when top is inherited from window.__proto__. It creates a new property on window that's a copy of the property on window.__proto__.)
Flash does an innocent JS_GetUCProperty(window, "top") which ends up in nsWindowSH::NewResolve with flags & JSRESOLVE_ASSIGNING. This triggers the following special case: if (flags & JSRESOLVE_ASSIGNING) { if (IsReadonlyReplaceable(id) || (!(flags & JSRESOLVE_QUALIFIED) && IsWritableReplaceable(id))) { // A readonly "replaceable" property is being set, or a // readwrite "replaceable" property is being set w/o being // fully qualified. Define the property on obj with the value // undefined to override the predefined property. This is done // for compatibility with other browsers. JSAutoRequest ar(cx); if (!::JS_DefineUCProperty(cx, obj, ::JS_GetStringChars(str), ::JS_GetStringLength(str), JSVAL_VOID, nsnull, nsnull, JSPROP_ENUMERATE)) { return NS_ERROR_FAILURE; } *objp = obj; return NS_OK; } Since we're *not* really assigning here, this causes the bogus window.top=undefined behavior. The flags are being erroneously inferred from the currently executing script, which is doing an assignment: e.innerHTML = '<embed ...>'; I don't yet know why this bug didn't happen before. Investigating. It's a longstanding dumb thing that the JSAPI examines the bytecode to get these flags when it shouldn't, and that seems worth fixing regardless of what else I find. Assigning window.top=window.top works around this by triggering this special NewResolve case earlier, then overwriting the (undefined) with the correct value of window.top. Once the property exists, the NewResolve hook doesn't get called again for that property.
I think mrbkap looked into this some time ago too, in a different bug most likely. I think he had ideas on how to deal with this...
(In reply to comment #5) > Assigning window.top=window.top works around this by triggering this special > NewResolve case earlier This explains what Adblock Plus has to do with it - it accesses window.top before Flash gets the chance...
(In reply to comment #7) > (In reply to comment #5) > > Assigning window.top=window.top works around this by triggering this special > > NewResolve case earlier > > This explains what Adblock Plus has to do with it - it accesses window.top > before Flash gets the chance... Still weird, though, since NoScript accesses window.top as well, both in content policy and in a global web progress listener...
(In reply to comment #8) > Still weird, though, since NoScript accesses window.top as well, both in > content policy and in a global web progress listener... Does NoScript assign to window.top? Does Adblock Plus? The full story also involves inner and outer window objects. - Flash asks for JS_GetUCProperty(outerWindow, "top") - the outer window's nsWindowSH::NewResolve is called (with erroneous resolve flags) - which forwards to the inner window's nsWindowSH::NewResolve - which creates the "top" property on the inner window If I comment out the nsIDOMWindow.top quick stub, |top| becomes undefined anyway... but |this.top| doesn't!... and then, a few seconds later, |this.top| becomes undefined too. Property cache? I still cannot explain what quick stubs have to do with any of this, but I'm in an interesting gdb session now.
As mentioned in comment 9, removing just the nsIDOMWindow.top quick stub does not fix the issue. But removing all quick stubs does, although not in the expected way: Flash then never asks for the "top" property. Quick stubs must affect some previous answer Flash gets from JS that changes its behavior. At this point I'd really like to talk to someone at Adobe who's familiar with the plugin code.
NoScript never assigns *to* window.top, it just reads it (possibly assigning it to local vars) in a few places. It's usually retrieved either from the context passed in nsIContentPolicy.shouldLoad() as context.ownerDocument.defaultView.top or in the wplistener callbacks as webProgress.DOMWindow.top. Regarding the Flash code, as far as I know it's an ugly hack used to guess current location for cross-domain security checks. Googling a bit, it appears to do something like window.location + "__flashplugin_unique__" || top.location + "__flashplugin_unique__" Therefore my guess is that the first location.toString() fails for whatever reason (I do remember seeing random Location.toString() exceptions in my Error Console sometimes) then it falls back on the second statement and breaks on undefined top.
If you mean "Error: uncaught exception: Permission denied to call method Location.toString" Then this is Bug 385676. See specifically Bug 385676 comment 7 which references bug 434522 and http://bugs.adobe.com/jira/browse/FP-561
jorendorff: on IRC unless I misunderstood (reading scrollback), you were looking for a place to funnel back into the interpreter from native code. If that place is not js_Interpret itself, then js_Execute and js_Invoke come to mind. Probably first I should ask: what did you learn in gdb? /be
The fix in bug 454343 also fixed this. (In reply to comment #13) > jorendorff: on IRC unless I misunderstood (reading scrollback), you were > looking for a place to funnel back into the interpreter from native code. If > that place is not js_Interpret itself, then js_Execute and js_Invoke come to > mind. Probably first I should ask: what did you learn in gdb? Not much. Comment 11 was more helpful. Back in comment 5 I mentioned wanting to fix some longstanding weirdness about resolve flags. The weirdness is bug 389034. I'll post my patch there.
Correction, this is not fixed. I was testing under the wrong profile.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Reopening based on Comment 15
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → NEW
Finally it makes sense. The quick stub that causes this is nsIDOMNSHTMLElement.innerHTML. Bug 389034 and quick stubs collided. Ordinary calls into XPConnect are via JSNatives and thus have a JSStackFrame (with no script). That prevents js_LookupPropertyWithFlags from peeking at bytecodes to infer JSRESOLVE flags; it only ever looks at the top frame. Quick stubs are JSPropertyOps or JSFastNatives. No JSStackFrame, so flags are inferred--even though there are about 20 C++ stack frames separating the .innerHTML= assignment from the lookup!
Depends on: 389034
Flags: blocking1.9.1?
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P3
Assignee: nobody → jorendorff
It appears fixing bug 389034 has fixed this as I hoped. Can anyone verify?
Status: NEW → RESOLVED
Closed: 17 years ago → 16 years ago
Resolution: --- → FIXED
I haven't seen this in a while, seems fixed to me.
I got a bug report that http://bludomaintemplates.com/adriana/index2.php doesn't work in Firefox 3.0.6 if Adblock Plus is installed (confirmed, seems only to happen if the page isn't cached). Same page works fine in Minefield. I see this error message in Error Console twice: Error: top is undefined Source File: javascript:top.location+"__flashplugin_unique__" Line: 1 Can it be the same issue?
I got a bug report from Symantec about http://www.symantec.com/business/security_response/landing/spam/ - the two graphs at the bottom don't load in Firefox 3.0.6 with Adblock Plus. Again, same error message: "top is undefined".
Given the number of reports recently, nominating as Firefox 3.0.8 blocker.
Flags: blocking1.9.0.8?
Comments 21 and 22 are hard to square with comment 1 saying this is a regression from bug 407216 which never landed on 1.9.0, and the perf fix that fixed this in 1.9.1 certainly isn't going to work on 1.9.0 as-is. Are you sure you're talking about the same bug? We really need a working testcase we can use, and a regression range. For now not blocking, but if it turns out to be a regression from a recent security fix we'll reconsider (but also please file as a separate bug if it's not due to 407216 and fixed by 389034).
Flags: blocking1.9.0.8?
Whiteboard: \
Filed bug 500715 on the 1.9.0 issue.
You need to log in before you can comment on or make changes to this bug.