[webkit-reviews] review granted: [Bug 21595] Clean up background/overflow propagation to match the latest 2.1 spec : [Attachment 24345] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 14 13:26:18 PDT 2008


Adam Roben (aroben) <aroben at apple.com> has granted Dave Hyatt
<hyatt at apple.com>'s request for review:
Bug 21595: Clean up background/overflow propagation to match the latest 2.1
spec
https://bugs.webkit.org/show_bug.cgi?id=21595

Attachment 24345: Patch
https://bugs.webkit.org/attachment.cgi?id=24345&action=edit

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
+		 RenderObject* o = rootRenderer->style()->overflowX() ==
OVISIBLE && document->documentElement()->hasTagName(HTMLNames::htmlTag) ?
body->renderer() : rootRenderer;

You shouldn't need the "HTMLNames::" prefix on htmlTag.

 141		 // Overflow on the body can potentially apply directly to the
body under the following conditions.
 142		 // (1) The root element is not <html>.
 143		 // (2) We are not the primary <body> (can be checked by
looking at document.body).
 144		 // (3) The root element already has overflow set, in which
case we don't propagate.
 145		 if
(document()->documentElement()->hasTagName(HTMLNames::htmlTag) &&
 146		     document()->body() == element() &&
 147		    
document()->documentElement()->renderer()->style()->overflowX() == OVISIBLE)
 148		     overflowApplies = false;

The comment and the code don't seem to agree. The code is saying "if the root
element is <html>, and we are the primary <body>, and the root element already
has overflow set, then don't propagate". That's not what the comment seems to
say. Which one is correct? Or am I just misinterpreting all of this (in which
case, how can we make it clearer)?

The "HTMLNames::" prefix shouldn't be needed here, either.

r=me


More information about the webkit-reviews mailing list