Summary: | Bundle lineLeftOffset and lineRightOffset as a class | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Enhancement | CC: | commit-queue, dglazkov, eric, hyatt, jamesr, leviw, mitz, simon.fraser | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | 57943 | ||||||||||
Bug Blocks: | 57779 | ||||||||||
Attachments: |
|
Description
Ryosuke Niwa
2011-04-05 08:17:02 PDT
Created attachment 88241 [details]
cleanup
Created attachment 88242 [details]
cleanup
Comment on attachment 88242 [details] cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=88242&action=review > Source/WebCore/rendering/RenderBlockLineLayout.cpp:80 > + extraWidth += borderPaddingMarginStart(toRenderBoxModelObject(parent)); I might have even added a RenderBoxModelObject* parentAsBoxModelObject local. I wonder it that's always true for SVG Text. I think we still use a RenderBoxModel object to hol the SVG text. I think we have to since we haven't split LineBoxTree out into its own class. > Source/WebCore/rendering/RenderBlockLineLayout.cpp:598 > +static inline bool isWhitespace(UChar character, bool shouldPreserveNewline) Might want a more specific name relating to what spec defines this whitespace, etc. isCSS3WhiteSpace for example. isCSSSkipableWhitespace, or wahtever. > Source/WebCore/rendering/RenderBlockLineLayout.cpp:2279 > +bool RenderBlock::positionNewFloatOnLine(FloatingObject* newFloat, FloatingObject* lastFloatFromPreviousLine, bool firstLine, LineOffsets& lineOffsets) I would have preferred that you moved this code in a separate patch. Makes it difficult to read this diff. I'm slightly lost in all the changes you're making. Created attachment 88413 [details]
cleanup
Comment on attachment 88413 [details]
cleanup
pretty.
(In reply to comment #5) > (From update of attachment 88413 [details]) > pretty. Yay! Thanks for the review. Hopefully, I can add more high-level methods in later patches. Comment on attachment 88413 [details] cleanup Clearing flags on attachment: 88413 Committed r83076: <http://trac.webkit.org/changeset/83076> All reviewed patches have been landed. Closing bug. |