Summary: | Rename Document::setContainsFullScreenElementRecursively | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||
Component: | WebCore Misc. | Assignee: | Jer Noble <jer.noble> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | darin, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Jer Noble
2011-06-12 22:21:30 PDT
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. |