Closed Bug 888225 Opened 7 years ago Closed 7 years ago
firefox 22 breaks access to frames named 'sidebar'
Completing the test case
I tried with FF21 nightlies, same error. Could you test with a clean profil, please. https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles
(In reply to Loic from comment #3) [...] > I tried with FF21 nightlies, same error. Argl. Simplified the test case too much and didn't notice (not sure but I guess mixed it up with Chrome, which I also tested with). I'll try (hopefully) on Monday to create a new test case which properly shows the problem I am (still) observing here. Sorry for the inconvenience. > Could you test with a clean profil, please. > https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove- > firefox-profiles I can confirm that I still observe the problems when I completely remove the .mozilla directory and start anew when using Firefox 22, but not with Firefox 21. As I said I will try to create a working test case soon.
Attachment #768959 - Attachment mime type: text/plain → text/html
Oh, I just noticed with the new testcase, the console error (TypeError: document.sidebar is undefined @ file:...demo.html:1) doesn't happen anymore. It seems that was only part of my faulty testcase. That is, the test case fail for Firefox 22 silently, while it works with Firefox 21.
Presumably the issue is that window.sidebar used to be shadowed by the frame name but now shadows the frame name... We had this issue with window.content too: see bug 857555. I guess we need to go through all the nonstandard properties on Window and do something like this...
For the record :) Regression range: good=2013-03-16 bad=2013-03-17 http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8f5b1f9f5804&tochange=0b052daa913c Bug 850517.
bz, can you make a list of nonstandard properties I should do this for? I could try to come up with such a list, but I think you'd probably be quicker and better at it. Let me know if you don't have time.
I definitely can't at least for another week. :( I wonder whether it's better to just push on with WebIDL Window and mark these as ChromeOnly or something....
(In reply to Boris Zbarsky (:bz) (reading mail, but on paternity leave) from comment #10) > I wonder whether it's better to just push on with WebIDL Window and mark > these as ChromeOnly or something.... That's certainly easier. Do we care enough about the web compat regression to want to backport a fix? If so, we need to do something like comment 9, because we sure aren't going to backport WebIDL Window. I'll defer to your (and alex's) judgement here. Thoughts?
I think it would be easier to answer that question if we knew the list of affected property names. I do think we should backport a fix for "sidebar".
Smaug, are you able to come up with such a list?
I'm not familiar with the changes done in Bug 850517.
The CSSOM view stuff is standard. find() is non-standard but commonly present in UAs, so not likely to be a performance issue. .performance is standard. move/resizeTo/By are commonly present in UAs. Most of the rest seem unlikely to collide with frame names. Let's just fix sidebar and external and call it good.
Comment on attachment 769580 [details] [diff] [review] Check for child window naming collisions before defining global properties in GlobalResolve. v1 r=me. Thank you!
Attachment #769580 - Flags: review?(bzbarsky) → review+
Comment on attachment 769580 [details] [diff] [review] Check for child window naming collisions before defining global properties in GlobalResolve. v1 [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 850517 User impact if declined: web compat regressions with any web page that has an iframe with name="sidebar" or name="external" and uses named subframe access (window['sidebar']). Testing completed (on m-c, etc.): Just pushed to m-i. Risk to taking this patch (and alternatives if risky): Low risk, just puts us back to the old behavior for these two property names. No alternatives. String or IDL/UUID changes made by this patch: None.
Do I understand correctly, that the bugfix is currently not intended to be applied to any version earlier than 25? If so, may I ask why? (the little I understand about the patch, it looks harmless enough to backport). PS: It doesn't matter for my site, because we couldn't wait for any fix anyhow and had to deploy a work-around.
The patch is requesting approval for Aurora and Beta. It means that the fix will be applied to Firefox 23 and 24 if approved.
Comment on attachment 769580 [details] [diff] [review] Check for child window naming collisions before defining global properties in GlobalResolve. v1 low risk Patch for a Fx22 regression that has a few web compat reports related to 'sidebar'. We Still have a few more beta cycles hence this is better to land asap.
OS: Linux → All
Hardware: x86_64 → All
Assuming this does not need QA verification due to in-testsuite coverage. Please remove [qa-] from the whiteboard and add the verifyme keyword if this needs to be manually verified.
You need to log in before you can comment on or make changes to this bug.