Open Bug 791605 Opened 13 years ago Updated 3 years ago

The string functions of hiddenDOMWindow are undefined

Categories

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

17 Branch
x86
Linux
defect

Tracking

()

Tracking Status
firefox17 - ---
firefox18 - ---

People

(Reporter: pzhang, Unassigned)

References

Details

(Keywords: regression)

These string functions are undefined, e.g., encodeURIComponent: Components.classes["@mozilla.org/appshell/appShellService;1"] .getService(Components.interfaces.nsIAppShellService) .hiddenDOMWindow.encodeURIComponent
It's OK on Firefox nightly and Firefox Beta.
OK, so where is it not OK on?
(In reply to Boris Zbarsky (:bz) from comment #2) > OK, so where is it not OK on? Aurora 17
OK. Would you be willing to find the one-day ranges when the problem appeared and disappeared?
I am using the PPA source on ubuntu: https://launchpad.net/~ubuntu-mozilla-daily/+archive/firefox-aurora I updated the Aurora last week, and one of my extensions broke because of this. Is this enough?
Rat, I was thinking more along the lines of using http://harthur.github.com/mozregression/
(In reply to Boris Zbarsky (:bz) from comment #6) > Rat, I was thinking more along the lines of using > http://harthur.github.com/mozregression/ Em, an interesting tool, let me have a try.
(In reply to Ray Cheung [:pzhang] from comment #7) > (In reply to Boris Zbarsky (:bz) from comment #6) > > Rat, I was thinking more along the lines of using > > http://harthur.github.com/mozregression/ > Em, an interesting tool, let me have a try. Hi Boris, the tool is trying to download the nightly build, but it's the problem on the Aurora, i am pretty sure it's bad on Aurora 2012-08-14, but how can i know which nightly build it is bad on?
See https://wiki.mozilla.org/RapidRelease/Calendar The last nightly before Aurora branched was August 27.
It's good on Aurora 2012-09-06, and bad on Aurora 2012-09-07
Ray, what are the build IDs (from about:buildconfig) for those two builds?
2012-09-07 ------------------------------------------------- about:buildconfig Build Machine linux64-ix-slave17 Source Built from http://hg.mozilla.org/releases/mozilla-aurora/rev/41eab5a0e537 Build platform target x86_64-unknown-linux-gnu Build tools Compiler Version Compiler flags /usr/bin/ccache /tools/gcc-4.5-0moz3/bin/gcc gcc version 4.5.2 (GCC) -pedantic -Wall -Wpointer-arith -Wdeclaration-after-statement -Werror=return-type -Wtype-limits -Wempty-body -Wno-unused -Wno-overlength-strings -Wcast-align -Wno-long-long -fno-strict-aliasing -ffunction-sections -fdata-sections -pthread -pipe -DNDEBUG -DTRIMMED -g -fprofile-use -fprofile-correction -Wcoverage-mismatch -O3 -fomit-frame-pointer /usr/bin/ccache /tools/gcc-4.5-0moz3/bin/g++ gcc version 4.5.2 (GCC) -pedantic -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Wno-ctor-dtor-privacy -Wno-overlength-strings -Wno-invalid-offsetof -Wno-variadic-macros -Wcast-align -Wno-long-long -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -std=gnu++0x -pthread -pipe -DNDEBUG -DTRIMMED -g -fprofile-use -fprofile-correction -Wcoverage-mismatch -O3 -fomit-frame-pointer Configure arguments --enable-update-channel=aurora --enable-update-packaging --enable-codesighs --enable-signmar --disable-profiling --enable-js-diagnostics --enable-stdcxx-compat --enable-warnings-as-errors --with-ccache=/usr/bin/ccache 2012-09-06 ------------------------------------------------ about:buildconfig Build Machine bld-centos5-64-vmw-003 Source Built from http://hg.mozilla.org/releases/mozilla-aurora/rev/c2cc63c864e4 Build platform target x86_64-unknown-linux-gnu Build tools Compiler Version Compiler flags /usr/bin/ccache /tools/gcc-4.5-0moz3/bin/gcc gcc version 4.5.2 (GCC) -pedantic -Wall -Wpointer-arith -Wdeclaration-after-statement -Werror=return-type -Wtype-limits -Wempty-body -Wno-unused -Wno-overlength-strings -Wcast-align -Wno-long-long -fno-strict-aliasing -ffunction-sections -fdata-sections -pthread -pipe -DNDEBUG -DTRIMMED -g -fprofile-use -fprofile-correction -Wcoverage-mismatch -O3 -fomit-frame-pointer /usr/bin/ccache /tools/gcc-4.5-0moz3/bin/g++ gcc version 4.5.2 (GCC) -pedantic -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Wno-ctor-dtor-privacy -Wno-overlength-strings -Wno-invalid-offsetof -Wno-variadic-macros -Wcast-align -Wno-long-long -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -std=gnu++0x -pthread -pipe -DNDEBUG -DTRIMMED -g -fprofile-use -fprofile-correction -Wcoverage-mismatch -O3 -fomit-frame-pointer Configure arguments --enable-update-channel=aurora --enable-update-packaging --enable-codesighs --enable-signmar --disable-profiling --enable-js-diagnostics --enable-stdcxx-compat --enable-warnings-as-errors --with-ccache=/usr/bin/ccache
Perfect, thanks. That would be this checkin range: http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=c2cc63c864e4&tochange=41eab5a0e537 Bobby, this looks likely to be fallout from one of your changes in that range... and something we then fixed for 18?
Keywords: regression
What's the user impact here if left unfixed?
(In reply to Alex Keybl [:akeybl] from comment #14) > What's the user impact here if left unfixed? The impact is the user can't use some extensions on Aurora, the user will have to switch browser or stop using the extensions. It's OK on Nightly, so it's not too hard to be fixed, right?
That depends on what fixed it on nightly. What's the mozregression range for the problem going away?
Aha, it also doesn't work on the Nightly 2012-09-17, i might tested that on an older version Nightly.
Hi Boris, the checking range for this regression on Nightly should be: http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2012-09-05&enddate=2012-09-06
Right, the regression range has the same patches as the aurora one. I was looking for the _fix_ range, when the claim was that it had been fixed on nightly...
(In reply to Ray Cheung [:pzhang] from comment #15) > (In reply to Alex Keybl [:akeybl] from comment #14) > > What's the user impact here if left unfixed? > The impact is the user can't use some extensions on Aurora, the user will > have to switch browser or stop using the extensions. > It's OK on Nightly, so it's not too hard to be fixed, right? This question is w/r/t the tracking nomination. We don't track all regressions, only those with user impact. It sounds like we think this will cause add-on compatibility issues.
The "Components" of the hiddenDOMWindow is not accessible either: Components.classes["@mozilla.org/appshell/appShellService;1"].getService(Components.interfaces.nsIAppShellService).hiddenDOMWindow.Components
This works fine for me on a sept 20 aurora build: http://hg.mozilla.org/releases/mozilla-aurora/rev/50e5a53ed557 I evaluated both testcases (comment 0 and comment 21) via the web console and a chrome scratchpad. The properties appear in all cases. Ray, can you still reproduce this issue? If so, where are you evaluating the code? Do you see it when you: (1) Go to about:config, and set |devtools.chrome.enabled| to true. (2) Open a scratchpad (3) Go to the |Environment| menu, and select |Browser| (4) Enter the following into the scratchpad: alert(Components.classes["@mozilla.org/appshell/appShellService;1"].getService(Components.interfaces.nsIAppShellService).hiddenDOMWindow.encodeURIComponent); (5) Run
(In reply to Bobby Holley (:bholley) from comment #22) > This works fine for me on a sept 20 aurora build: > http://hg.mozilla.org/releases/mozilla-aurora/rev/50e5a53ed557 > > I evaluated both testcases (comment 0 and comment 21) via the web console > and a chrome scratchpad. The properties appear in all cases. > > Ray, can you still reproduce this issue? If so, where are you evaluating the > code? Do you see it when you: > (1) Go to about:config, and set |devtools.chrome.enabled| to true. > (2) Open a scratchpad > (3) Go to the |Environment| menu, and select |Browser| > (4) Enter the following into the scratchpad: > alert(Components.classes["@mozilla.org/appshell/appShellService;1"]. > getService(Components.interfaces.nsIAppShellService).hiddenDOMWindow. > encodeURIComponent); > (5) Run Yes, I can reproduce the problem, I am using Aurora 2012-09-16. I evaluated the code in the Error Console.
(In reply to Ray Cheung [:pzhang] from comment #23) > Yes, I can reproduce the problem, I am using Aurora 2012-09-16. > I evaluated the code in the Error Console. Can you reproduce it on the sept 20 build above, or a later build?
(In reply to Bobby Holley (:bholley) from comment #24) > (In reply to Ray Cheung [:pzhang] from comment #23) > > Yes, I can reproduce the problem, I am using Aurora 2012-09-16. > > I evaluated the code in the Error Console. > > Can you reproduce it on the sept 20 build above, or a later build? I tried on this build: ftp://ftp.mozilla.org/pub/firefox/nightly/2012/09/2012-09-20-04-20-10-mozilla-aurora/firefox-17.0a2.en-US.linux-x86_64.tar.bz2 , it still has the problem.
Adding qawanted - can someone in QA figure out why Ray can reproduce this but I can't? I'm guessing it's a a linux-only thing (I'm testing on OSX), but it'd be helpful to have someone confirm that with a linux box before I go rehydrating my defunct VM. STR are in comment 22.
Keywords: qawanted
(In reply to Bobby Holley (:bholley) from comment #26) > I'm guessing it's a a linux-only thing I can confirm that the steps in comment 22 yield undefined on Ubuntu and not on Mac OS (function encodeURIcomponent...) Tried with Aurora The dubious part, however, is that I can reproduce this on Ubuntu in F16, 17, Aurora and Nightly. And can't reproduce in F12, so whatever my regression would be is different from the initial one.
QA Contact: virgil.dicu
This only affects Linux only, and we have no known actual add-on impact. There are bigger fish to fry right now.
(please renominate if the situation in Comment 28 changes)
Peter is going to look at this. Peter, note that this reproduces only on linux, I think.
Assignee: nobody → phx
(In reply to Bobby Holley (:bholley) from comment #30) > Peter is going to look at this. Peter, note that this reproduces only on > linux, I think. I can't reproduce this on my Mac at least. I'll try on my linux box when I get back from my trip.
FTR, I can reproduce and get the same range as in comment 18 on a Ubuntu laptop. My work station shows misleading results, for don't know what reason. http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2012-09-05&enddate=2012-09-06
Keywords: qawanted
Sorry for being MIA, I've been pretty busy with school lately. Can I get access to bug 774633? I think the culprit might be in here. My plan of attack right now is to just to familiarize myself with the changes made in bug 774633.
(In reply to Peter Xiao from comment #33) > My plan of attack right now is to just to familiarize myself with the > changes made in bug 774633. I've CCed you. But note that those are some pretty big patches! I'd suggest doing the following: 1 - Confirm that you can reproduce the bug on a Nightly linux build. There appears to be some confusion in this bug over whether this affects mozilla-central or whether it only affected FF18. If the bug is fixed on Nightly, there's probably no point in working on it. 2 - Using bisect, narrow down which of the patches in bug 774633 broke this. My money would probably be on part 6. Unfortunately, that doesn't really tell us a whole lot, because that's a pretty wide-reaching change. 3 - Figure out and understand how this works on Mac/Windows, since those don't appear to be affected. 4 - Figure out what goes wrong on Linux. :-)
(In reply to Bobby Holley (:bholley) from comment #34) > 1 - Confirm that you can reproduce the bug on a Nightly linux build. There > appears to be some confusion in this bug over whether this affects > mozilla-central or whether it only affected FF18. If the bug is fixed on > Nightly, there's probably no point in working on it. I can reproduce this in a recent pull of mozilla-central. > 2 - Using bisect, narrow down which of the patches in bug 774633 broke this. > My money would probably be on part 6. Unfortunately, that doesn't really > tell us a whole lot, because that's a pretty wide-reaching change. As it turns out, the patch for bug 771354 broke this. Is the reason why we don't need to special case on chrome windows anymore related to bug 774633? > 3 - Figure out and understand how this works on Mac/Windows, since those > don't appear to be affected. Is it true to say that the document needs the SystemPrincipal to call encodeURIComponent? I've at least verified that on the Mac version, the document does have the SystemPrincipal. I don't know yet if the Linux version has SystemPrincipal, or why it would be different. > 4 - Figure out what goes wrong on Linux. :-) I spent too much time building so I haven't had the time to investigate this yet. I'll break out the gdb after my finals are done tomorrow. I will hopefully be able to confirm that
(In reply to Peter Xiao from comment #35) > > 2 - Using bisect, narrow down which of the patches in bug 774633 broke this. > > My money would probably be on part 6. Unfortunately, that doesn't really > > tell us a whole lot, because that's a pretty wide-reaching change. > > As it turns out, the patch for bug 771354 broke this. Is the reason why we > don't need to special case on chrome windows anymore related to bug 774633? Yeah. Before I cleaned things up in bug 774633, there was all sorts of voodoo and special-casing that determined what principal would end up on the compartment. I simplified this significantly, which meant that bug 771354 was possible. And this was good, because the code before bug 771354 was scary. Note that "chrome window" in this sense refers to a window implemented with nsGlobalChromeWindow rather than nsGlobalWindow. It's generally intended to hold chrome:// documents (which run with system principal), but it can sometimes end up displaying unprivileged stuff. So we definitely want to make sure we're getting the principal correct here. ;-) > > 3 - Figure out and understand how this works on Mac/Windows, since those > > don't appear to be affected. > > Is it true to say that the document needs the SystemPrincipal to call > encodeURIComponent? I've at least verified that on the Mac version, the > document does have the SystemPrincipal. I don't know yet if the Linux > version has SystemPrincipal, or why it would be different. Nope! encodeURIComponent is technically part of the Javascript specification (that is to say, it exists independently of anything DOM-y or browser-y). All web pages certainly have access to it. You can try it yourself like this: http://bit.ly/WjRBnU So the fact that it doesn't appear for Linux is really weird... What happens if, instead of hiddenDOMWindow.encodeURIComponent, you do hiddenDOMWindow.wrappedJSObject.encodeURIComponent?
Blocks: 771354
(In reply to Bobby Holley (:bholley) from comment #36) > What happens if, instead of hiddenDOMWindow.encodeURIComponent, you do > hiddenDOMWindow.wrappedJSObject.encodeURIComponent? This works! What does this mean?
(In reply to Peter Xiao from comment #37) > (In reply to Bobby Holley (:bholley) from comment #36) > > What happens if, instead of hiddenDOMWindow.encodeURIComponent, you do > > hiddenDOMWindow.wrappedJSObject.encodeURIComponent? > > This works! Ahah! > What does this mean? It means that that the caller is being given Xray vision into the hiddenDOMWindow (wrappedJSObject waives Xray vision). You can read all about Xrays her: https://blog.mozilla.org/gabor/2012/04/18/security-wrappers-part-1/ . You can also listen to me talk about it at length here: http://vreplay.mozilla.com/replay/showRecordingExternal.html?key=gmlj5y6slGfuBrd This makes sense, given the changeset you isolated. Previously, we were unconditionally using system principal for chrome windows (which the hiddenDOMWindow certainly is). Now, we use the principal for the Document we're creating the global for (much safer!), which you can see happening in nsGlobalWindow::SetNewDocument and CreateNativeGlobalForInner. So what you said in comment 35 was correct, in a way. If your code is running with system principal, you'll only see encodeURIComponent on scopes whose documents are also system principal, otherwise the Xray will obscure it (which it is known to do, though shouldn't - a separate bug, but let's not get sidetracked). This is weird though, because it implies that, on linux, the hidden DOM window has a non-system principal, which shouldn't happen (the URI is chrome://browser/content/hiddenWindow.xul, which should be system principal). Can you verify the principal being used for the hidden DOM window? Try breaking in nsAppShellService::CreateHiddenWindow, then breaking in nsGlobalWindow::SetNewDocument to see what principal gets used on the eventual loaded document. Make sure you're looking at the actual document and not a transient about:blank document that might appear along the way.
Ahah! bz pointed out to me on IRC that I skimmed this line too quickly: http://mxr.mozilla.org/mozilla-central/source/xpfe/appshell/src/nsAppShellService.cpp#89 So there's actually an #ifdef XP_MACOSX in there. For Mac, we use the chrome:// URI. For non-mac, we use a file that lives in resource://, which is unprivileged. From IRC: bz: This is why One Should Not Use The Hidden Window (TM)
bsmedberg: The simplest solution to this bug would be to move the non-mac hidden window from resource:// to chrome://. It looks like there was some shuffling of it in bug 375861, but it __appears__ that the default one has always been resource://, presumably because it's an HTML file. Do you think moving the file has the potential to cause embedding breakage?
Flags: needinfo?(benjamin)
This is why one should not use the hidden window! Can we just get rid of the hidden window completely on linux/windows let/force clients make their own private hidden windows if they need to do weird shit? The purpose of the appshellservice hidden window is to control the menus on mac when there is no other active window. It also used to hold the "safe JS context", but I believe that we got rid of that a while back, and now we shouldn't even have an appshellservice hidden window on non-mac platforms. Please! Changes to the hidden window probably won't affect embedders at all. They will affect addon authors, and I cannot really predict the ways in which changes might affect addons. I think that jetpack uses the hidden window as a storage location for some kinds of content, but I'm not sure exactly what.
Flags: needinfo?(benjamin)
> Can we just get rid of the hidden window completely on linux/windows That would break the Add-on SDK, which abuses the hidden window for panels and stuff. :( > let/force clients make their own private hidden windows if they need to do weird shit? See bug 565388. I've been trying to find time for the last week to put something together there... but until I do, there's no scriptable way to create private hidden windows that I can see. :( > but I believe that we got rid of that a while back Yes, we did.
Anyway, sounds like a good solution for this bug is to move hiddenWindow.html to chrome. Want to take a stab at it, Peter?
(In reply to Bobby Holley (:bholley) from comment #43) > Anyway, sounds like a good solution for this bug is to move > hiddenWindow.html to chrome. Want to take a stab at it, Peter? Sure! A few questions: How does the hiddenWindow.html file get placed in chrome:// or resource://? It looks like there are two of them in my obj-ff folder, one in /dist/bin/chrome/toolkit/res/ and the other /dist/NightlyDebug.app/Contents/MacOS/chrome/toolkit/res/. I assume the second is for MacOS. Do resource://gre-resources/hiddenWindow.html and chrome://gre-resources/hiddenWindow.html reference the same file? And the only difference is that the latter signifies chrome privileges? (Also, are there docs on this?) Should I use the already existing chrome://browser/content/hiddenWindow.xul?
(In reply to Peter Xiao from comment #44) > (In reply to Bobby Holley (:bholley) from comment #43) > > Anyway, sounds like a good solution for this bug is to move > > hiddenWindow.html to chrome. Want to take a stab at it, Peter? > > Sure! > > A few questions: > > How does the hiddenWindow.html file get placed in chrome:// or resource://? > It looks like there are two of them in my obj-ff folder, one in > /dist/bin/chrome/toolkit/res/ and the other > /dist/NightlyDebug.app/Contents/MacOS/chrome/toolkit/res/. I assume the > second is for MacOS. dist/bin is the canonical place for stuff to go. A lot of stuff gets copied or symlinked into X.app for Mac. > Do resource://gre-resources/hiddenWindow.html and > chrome://gre-resources/hiddenWindow.html reference the same file? And the > only difference is that the latter signifies chrome privileges? (Also, are > there docs on this?) What shows up in different special URIs (chrome://, resource://, etc) depends on various mappings laid out in manifests. There are some docs here: https://developer.mozilla.org/en-US/docs/JAR_Manifests But it might just be simpler to look at how this works for hiddenWindow.xul, and make the same thing happen for hiddenWindow.html. Use MXR to search for references to "hiddenWindow.xul". > Should I use the already existing chrome://browser/content/hiddenWindow.xul? hiddenWindow.xul is the one we use on Mac, because it does fancy stuff to keep the menu bar displayed even when you don't have any windows open. We want to use hiddenWindow.xul for Mac, and hiddenWindow.html for everything else.
I changed jar.mn so that "content toolkit toolkit/res/ contentaccessible=yes" is added in the toolkit.manifest file. This means "chrome://toolkit/hiddenWindow.html" should exist. Now all I have to do is replace DEFAULT_HIDDENWINDOW_URL with this new URL, and everything should work? I would test this out on my Ubuntu machine, but I've been having issues with it. It keeps freezing during a build. I think I'll spend some time migrating my Wubi to a real partition. I thought I could test this on my Mac by setting browser.hiddenWindowChromeURL to chrome://toolkit/hiddenWindow.html, but Firefox doesn't even start up. I guess it's really important to have the hiddenWindow.xul for Mac.
(In reply to Peter Xiao from comment #46) > I changed jar.mn so that "content toolkit toolkit/res/ > contentaccessible=yes" is added in the toolkit.manifest file. This means > "chrome://toolkit/hiddenWindow.html" should exist. Why contentAccessible? > Now all I have to do is replace DEFAULT_HIDDENWINDOW_URL with this new URL, > and everything should work? Hopefully! :-) > I would test this out on my Ubuntu machine, but I've been having issues with > it. It keeps freezing during a build. I think I'll spend some time migrating > my Wubi to a real partition. Yikes. Yeah, this will definitely need to be smoketested. > I thought I could test this on my Mac by setting > browser.hiddenWindowChromeURL to chrome://toolkit/hiddenWindow.html, but > Firefox doesn't even start up. I guess it's really important to have the > hiddenWindow.xul for Mac. Heh, yeah. This is very deep magic we're playing with here. ;-)
(In reply to Bobby Holley (:bholley) from comment #47) > Why contentAccessible? Oops, I don't think we need it. The resource version doesn't have it. Also, does this change warrant a test?
(In reply to Peter Xiao from comment #48) > Also, does this change warrant a test? Yeah, should be pretty simple, though. Just write a mochitest-chrome test that checks that: hiddenDOMWindow.document.nodePrincipal.origin == "[System Principal]" (this proves that the hidden DOM window is running with system principal) and XPCNativeWrapper.unwrap(hiddenDOMWindow) === hiddenDOMWindow (this proves that XPCNativeWrapper.unwrap is a no-op on hiddenDOMWindow, which means it's not an Xray wrapper) do: perl testing/mochitest/gen_template.pl -type chrome -b 791605 to generate the boilerplate here. There's no directory for appshell tests, so put it in widget/tests.
I switched an actual partition of Ubuntu, recompiled with the above changes. Unfortunately, it still freezes on a test for about 20 minutes, but it finishes. Firefox now does not start up. I recompiled with debugging turned on, and got the following error: http://pastebin.com/GhbQv6ms. Any ideas on what I'm doing wrong?
Are you running the entire mochitest suite locally? There's no reason to do that, and sometimes there's weirdness. As long as the browser boots up fine, just do TEST_PATH=relative/path/of/the/tests/you/care/about make mochitest-{plain,chrome,browser-chrome} As for the startup issue. Is it a clean build? Can you pastebin your .mozconfig?
I'm just running ./mach build. Does that run the tests as well? Doing the same thing in OSX seems to take less than two minutes to compile. I believe it is a clean build. Here is the .mozconfig http://pastebin.mozilla.org/2061471. I forget where I copied it from.
(In reply to Peter Xiao from comment #52) > I'm just running ./mach build. Does that run the tests as well? It shouldn't. I was confused when you said: (In reply to Peter Xiao from comment #50) > it still freezes on a test for about 20 minutes, but it finishes. Which seemed to suggest that you were running the entire suite. That mozconfig looks reasonable. Do you have any patches applied, or is it a clean pull? Try nuking your objdir, rebuilding, and then cd-ing into objdir/dist/bin and running ./firefox -ProfileManager (make sure to create a clean profile so that you don't conflict with your currently-running version of Firefox)
Component: DOM: Other → DOM
Priority: -- → P5
Component: DOM → DOM: Core & HTML
Severity: normal → S3

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Assignee: phx → nobody
You need to log in before you can comment on or make changes to this bug.