Bug 61755 – Page layout messed up after exiting full screen at Apple trailers page

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

Page layout messed up after exiting full screen at Apple trailers page

[Jer Noble]

Created attachment 95388 [details]
Patch

[Darin Adler]

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]

(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 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]

<rdar://problem/9525277>

[Jer Noble]

Created attachment 95516 [details]
Patch

[Jer Noble]

Double-period in the ChangeLog.  Will fix before committing or in another patch.

[Darin Adler]

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]

Committed r87844: <http://trac.webkit.org/changeset/87844>