Summary: | [CSSRegions] Make RenderFlowThread track if it has variable width regions | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dave Hyatt <hyatt> | ||||||
Component: | Layout and Rendering | Assignee: | Dave Hyatt <hyatt> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | aroben | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Dave Hyatt
2011-09-22 10:52:39 PDT
Created attachment 108359 [details]
Patch
Created attachment 108361 [details]
Patch
Comment on attachment 108361 [details]
Patch
r=me
I think the naming can be improved, “variable widths” seems like the width can change, maybe you can use “nonUniformLogicalWidth” or flip it and call it “regionsHaveUniformLogicalWidth”.
Comment on attachment 108361 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108361&action=review > Source/WebCore/rendering/RenderFlowThread.cpp:308 > + m_hasValidRegions = false; > + LayoutUnit previousRegionLogicalWidth = 0; Seems like you need to set m_regionsHaveVariableLogicalWidth to false here, too. Otherwise once it goes true it will never become false again! > Source/WebCore/rendering/RenderFlowThread.h:108 > + bool regionsHaveVariableLogicalWidth() const { return m_regionsHaveVariableLogicalWidth; } Seems like this isn't needed yet, but maybe you intend to use it in the future? (In reply to comment #4) > Seems like you need to set m_regionsHaveVariableLogicalWidth to false here, too. Otherwise once it goes true it will never become false again! > Nice catch! Wouldn't be a correctness issue, but would be a performance one! |