Summary: | Page layout messed up after exiting full screen at Apple trailers page | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||||
Component: | New Bugs | Assignee: | Jer Noble <jer.noble> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin, jeffm | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Jer Noble
2011-05-30 17:51:28 PDT
Created attachment 95388 [details]
Patch
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? (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." 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. Created attachment 95516 [details]
Patch
Double-period in the ChangeLog. Will fix before committing or in another patch. 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.
Committed r87844: <http://trac.webkit.org/changeset/87844> |