Document::setContainsFullScreenElementRecursively is named poorly, as it does not actually use recursion. Also, make explicit that the function walks up the ancestor chain and crosses frame boundaries. (See Darin's comments in https://bugs.webkit.org/show_bug.cgi?id=62507#c5.)
Created attachment 96933 [details] Patch
In the end, I made setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries() a member function in Element (alongside setContainsFullScreenElement()), so it could be used so: m_fullScreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(true).
Comment on attachment 96933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96933&action=review > Source/WebCore/dom/Element.cpp:996 > - if (parentElement() && containsFullScreenElement() && !parentElement()->containsFullScreenElement()) > - document()->setContainsFullScreenElementRecursively(parentElement(), true); > + if (containsFullScreenElement() && parentElement() && !parentElement()->containsFullScreenElement()) > + setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(true); I don’t think the additional checks in this if statement are needed or helpful. I see no compelling reason to optimize for this case, and it seems it could cause problems if the element being inserted is the top level element; we’ll miss out on the code that follows the ownerElement pointer. But it may be that the top level element is never inserted into a tree at a time where this matters. > Source/WebCore/dom/Element.cpp:1931 > +void Element::setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(bool flag) I probably would not make this a member function, just because I don’t want to have to modify Element.h every time I change it. But I think it’s OK as one.
Comment on attachment 96933 [details] Patch Clearing flags on attachment: 96933 Committed r89205: <http://trac.webkit.org/changeset/89205>
All reviewed patches have been landed. Closing bug.