Closed Bug 127013 Opened 19 years ago Closed 13 years ago
Screen property operates inconsistently for frames
*** Bug 127440 has been marked as a duplicate of this bug. ***
Neil: since bug 127405 was fixed, we can't do fullScreen = true/false outside mozilla now. should we close this bug?
http://lxr.mozilla.org/seamonkey/source/dom/src/base/nsGlobalWindow.cpp#1884 We shouldn't do that before we are the root window.
Assignee: hewitt → kyle.yuan
Comment on attachment 92368 [details] [diff] [review] checks (aFullScreen == mFullScreen) after we were the root window Nice work but can you fix GetFullScreen as well?
Attachment #92368 - Flags: needs-work+
Comment on attachment 92506 [details] [diff] [review] changed GetFullScreen as well (per Neil's suggestion) this patch doesn't solve the problem.
Attachment #92506 - Attachment is obsolete: true
=> default owner
Assignee: kyle.yuan → jaggernaut
*** Bug 227553 has been marked as a duplicate of this bug. ***
So just to be clear here, there are three code paths: 1. setting full screen forwards to top 2. unsetting full screen early returns when it shouldn't (except for top) 3. getting full screen always reads the local member (wrong except for top) Would it be better to store the full screen state somewhere else?
Summary: fullScreen = false doesn't turn full screen mode off → fullScreen property operates inconsistently for frames
Even better: we read the full screen flag using the same approach as setting it, and then just re-use it in setFullScreen. Not only does this fix Neil's problem, but this fixes the broken readonly window.fullScreen for content (which I'm sure the many upcoming web-based Powerpoint killers can use ;) )
Comment on attachment 264942 [details] [diff] [review] New patch Looks good. r+sr=jst
Michael, you need someone to check the patch in for you? Also, I guess this needs some Mochikit tests, eventually: http://developer.mozilla.org/en/docs/Mochitest
(In reply to comment #14) > Michael, you need someone to check the patch in for you? Yes please. > Also, I guess this needs some Mochikit tests, eventually: > http://developer.mozilla.org/en/docs/Mochitest I'll see what I can do.
Whiteboard: [checkin needed]
Now that the tree has reopened, can someone with CVS rights please check-in this patch?
Checking in nsGlobalWindow.cpp; /cvsroot/mozilla/dom/src/base/nsGlobalWindow.cpp,v <-- nsGlobalWindow.cpp new revision: 1.936; previous revision: 1.935 done Checked into trunk. Thanks for the patch Michael! Also thank you for the Mochitest! I'll review that one.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Comment on attachment 266033 [details] [diff] [review] Mochitest Thanks!
I noticed you didn't reset the window back after the test, so I added it. I also added a test whether the resize event is fired. I hope you don't mind I did this. I added some very limited documentation here: http://developer.mozilla.org/en/docs/DOM:window.fullScreen
(In reply to comment #21) > Created an attachment (id=267220) [details] > Mochitest v2 > > I noticed you didn't reset the window back after the test, so I added it. I > also added a test whether the resize event is fired. I hope you don't mind I > did this. That's fine. In fact I thought about that just after I hit submit, but never got around to fixing it.
You need to log in before you can comment on or make changes to this bug.