Bug 21595 - Clean up background/overflow propagation to match the latest 2.1 spec
Summary: Clean up background/overflow propagation to match the latest 2.1 spec
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-14 12:34 PDT by Dave Hyatt
Modified: 2008-10-14 13:35 PDT (History)
1 user (show)

See Also:


Attachments
Patch (1.71 MB, patch)
2008-10-14 12:38 PDT, Dave Hyatt
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 2008-10-14 12:34:46 PDT
This bug is about cleaning up our background and overflow propagation to match the spec (and to pass test cases).  We need to start propagating in XHTML now in addition to HTML, and we also had lots of bugs with it that test cases caught.
Comment 1 Dave Hyatt 2008-10-14 12:38:10 PDT
Created attachment 24345 [details]
Patch
Comment 2 Adam Roben (:aroben) 2008-10-14 13:26:18 PDT
Comment on attachment 24345 [details]
Patch

+                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
Comment 3 Dave Hyatt 2008-10-14 13:35:56 PDT
Fixed in r37591.