RESOLVED FIXED 61755
Page layout messed up after exiting full screen at Apple trailers page
https://bugs.webkit.org/show_bug.cgi?id=61755
Summary Page layout messed up after exiting full screen at Apple trailers page
Jer Noble
Reported 2011-05-30 17:51:28 PDT
Page layout messed up after exiting full screen at Apple trailers page
Attachments
Patch (33.27 KB, patch)
2011-05-30 18:16 PDT, Jer Noble
no flags
Patch (33.31 KB, patch)
2011-05-31 17:17 PDT, Jer Noble
darin: review+
Jer Noble
Comment 1 2011-05-30 18:16:35 PDT
Darin Adler
Comment 2 2011-05-30 20:24:35 PDT
Comment on attachment 95388 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95388&action=review > Source/WebCore/ChangeLog:11 > + Incomplete implementation of r87660. Exiting full screen must set the "containsFullScreenElement" > + flag of every element in the full screen element's ancestors, not just the document's owner. This doesn’t make sense. Why would we do anything with the document’s ownerElement? That’s the <object> or <iframe> element in the parent document that owns this document in the case of a subframe. And why is starting with the full-screen element’s parent right? What about its grandparent? Or further up in the document?
Jer Noble
Comment 3 2011-05-30 20:38:50 PDT
(In reply to comment #2) > (From update of attachment 95388 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=95388&action=review > > > Source/WebCore/ChangeLog:11 > > + Incomplete implementation of r87660. Exiting full screen must set the "containsFullScreenElement" > > + flag of every element in the full screen element's ancestors, not just the document's owner. > > This doesn’t make sense. Why would we do anything with the document’s ownerElement? That’s the <object> or <iframe> element in the parent document that owns this document in the case of a subframe. > > And why is starting with the full-screen element’s parent right? What about its grandparent? Or further up in the document? Whatever point we start at, we have to match the call in Document::webkitWillEnterFullScreenForElement. Right now in that function, we start at the element's parentElement(), or if that's NULL, the document's ownerElement(). So the equivalent call in Document::webkitWillExitFullScreenForElement must match. So I guess a better comment here would be: "Make parameters to setContainsFullScreenElementRecursively() in webkitWillExitFullScreenForElement() match those in webkitWillEnterFullScreenForElement(), so the ancestors' flags do not become inconsistent."
Jeff Miller
Comment 4 2011-05-31 11:02:04 PDT
Comment on attachment 95388 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95388&action=review > Source/WebCore/ChangeLog:6 > + https://bugs.webkit.org/show_bug.cgi?id=61755 Please include a reference to the Radar bug as well.
Jer Noble
Comment 5 2011-05-31 17:16:11 PDT
Jer Noble
Comment 6 2011-05-31 17:17:12 PDT
Jer Noble
Comment 7 2011-05-31 17:19:00 PDT
Double-period in the ChangeLog. Will fix before committing or in another patch.
Darin Adler
Comment 8 2011-06-01 09:03:43 PDT
Comment on attachment 95516 [details] Patch I guess this change is OK, but both the parent of the full screen element and the owner element seem wrong. I really don't get it.
Jer Noble
Comment 9 2011-06-01 13:57:27 PDT
Note You need to log in before you can comment on or make changes to this bug.