Bug 21595

Summary: Clean up background/overflow propagation to match the latest 2.1 spec
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: CSSAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
Patch aroben: review+

Dave Hyatt
Reported 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.
Attachments
Patch (1.71 MB, patch)
2008-10-14 12:38 PDT, Dave Hyatt 		aroben: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Dave Hyatt
Comment 1 2008-10-14 12:38:10 PDT
Created attachment 24345 [details] Patch
Adam Roben (:aroben)
Comment 2 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
Dave Hyatt
Comment 3 2008-10-14 13:35:56 PDT
Fixed in r37591.
Note You need to log in before you can comment on or make changes to this bug.