Closed Bug 776129 Opened 9 years ago Closed 9 years ago
When mozbrowser does window
.open, the opened window should be OOP iff the opener is OOP
UI Test App window.open comes up with a rounded rectangle white window, much like bug 776086
All core apps need to run OOP
blocking-basecamp: --- → ?
The problem was that we weren't forcing the frame created by window.open to run out-of-process! This fixes things, but I need to figure out how to write a test, and also whether we have the inverse problem: Can in-process apps attempt to spawn out-of-process popups? That would also be bad.
Assignee: nobody → justin.lebar+bug
Setting remote=true for OOP popups is neither necessary nor sufficient to fix the OOP bug; I set the attribute only because we have to set it in the in-process case, and it would be weird for content to see remote=false but never see remote=true. Setting remote=true doesn't do anything because our OOP loading code (nsFrameLoaer::SetRemoteBrowser) doesn't ever call nsFrameLoader::MaybeCreateDocShell(), which is what would end up reading the remote attribute. We could change the frameloader code to call MaybeCreateDocShell(), if you preferred, but I think this is clearer.
Attachment #652474 - Flags: review?(bugs)
Attachment #652439 - Attachment is obsolete: true
Summary: UI Test - window.open doesn't work properly when run OOP → When mozbrowser does window.open, the opened window should be OOP iff the opener is OOP
Funny, I'm sitting on a patch to do this for bug 781725 :).
Comment on attachment 652474 [details] [diff] [review] Patch, v1 I think CreateIframe should take 3rd parameter to say whether the element is remote or not and set the attribute inside CreateIframe. With that, r=me
Attachment #652474 - Flags: review?(bugs) → review+
Comment on attachment 652475 [details] [diff] [review] Tests, v1 >+// We need to open OOP frames when the OOP-by-default pref is /off/, and we >+// similarly need to open in-process frames when the OOP-by-default pref is >+// /on/. >+// >+// Since the name of the test determines the OOP-by-default pref, the "inproc" >+// version of this test opens an OOP frame, and the "oop" version opens an >+// in-process frame. Enjoy. :) This is über-confusing. Could you explain *why* OOP /off/ means need-to-open-OOP. It seems that that is just what you're testing, but please say so. /me crosses fingers this actually passes on try ;)
Attachment #652475 - Flags: review?(bugs) → review+
> Could you explain *why* OOP /off/ means need-to-open-OOP. You're right, I should be more explicit in the bug and say that it appears when you create an OOP frame when oop-by-default == false, or when you create an inproc frame when oop-by-default == true. Thanks for the reviews!
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Hi Justin, could you please take a look at bug 778300? window.open() still doesn't work properly for us when calling it from an OOP app (Alarm-Clock). Is that because of the wrong calling of window.open()?
Alarm should be a notification not window.open.
(In reply to Andreas Gal :gal from comment #14) > Alarm should be a notification not window.open. Really? Last time I checked, we showed the incoming call screen using window.open. The alarm-is-sounding screen sounds pretty similar, at least at first glance.
I guess up to the UX design. In any case, the underlying bug has to be fixed since it will hit the dialer case.
You need to log in before you can comment on or make changes to this bug.