Summary: | [CSS Exclusions] Support outside-shape layout for shape-inside property | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Bear Travis <betravis> | ||||||||||
Component: | CSS | Assignee: | Bear Travis <betravis> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | donggwan.kim, eric, ojan.autocc, rego+ews, webkit-ews, webkit.review.bot, xan.lopez | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 101108 | ||||||||||||
Bug Blocks: | 89256, 109605 | ||||||||||||
Attachments: |
|
Description
Bear Travis
2012-11-16 16:23:29 PST
Also shift to use outside-shape as the default value for shape-inside. Created attachment 187916 [details] Initial patch Implementing outside-shape layout, waiting to make it the default value until bug 109605. Comment on attachment 187916 [details] Initial patch Attachment 187916 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/16483373 Created attachment 187921 [details]
Fixing typo
Comment on attachment 187921 [details] Fixing typo Attachment 187921 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/16440425 Comment on attachment 187921 [details] Fixing typo Attachment 187921 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/16440427 Created attachment 187927 [details]
Updating patch
Comment on attachment 187927 [details] Updating patch View in context: https://bugs.webkit.org/attachment.cgi?id=187927&action=review r- > Source/WebCore/rendering/ExclusionShapeInfo.cpp:49 > ExclusionShapeValue* shapeValue = (m_renderer->style()->*shapeGetter)(); > + if (shapeValue && shapeValue->type() == ExclusionShapeValue::OUTSIDE) > + shapeValue = m_renderer->style()->shapeOutside(); Couldn't you just fold all this into a single getter method rather than having to call another method for OUTSIDE? > Source/WebCore/rendering/ExclusionShapeInsideInfo.h:64 > ExclusionShapeValue* shapeValue = renderer->style()->shapeInside(); > + if (shapeValue && shapeValue->type() == ExclusionShapeValue::OUTSIDE) > + shapeValue = renderer->style()->shapeOutside(); Same question here. Really feels like you could create a method that does this so that the call site doesn't have to special case. > Source/WebCore/rendering/RenderBlock.cpp:1378 > + const ExclusionShapeValue* shapeInside = newStyle->shapeInside(); > + if (shapeInside && shapeInside->type() == ExclusionShapeValue::OUTSIDE) > + shapeInside = newStyle->shapeOutside(); Same complaint here. Don't be afraid to add a helper to RenderStyle that gives back the correct value and hides this. > Source/WebCore/rendering/RenderBlock.cpp:1381 > + if (oldShapeInside && oldShapeInside->type() == ExclusionShapeValue::OUTSIDE) > + oldShapeInside = oldStyle->shapeOutside(); Same here. Created attachment 188439 [details]
Adding resolvedShapeInside to RenderStyle
Consolidating repeated code into a helper method in RenderStyle.
Comment on attachment 188439 [details]
Adding resolvedShapeInside to RenderStyle
r=me
Comment on attachment 188439 [details] Adding resolvedShapeInside to RenderStyle Clearing flags on attachment: 188439 Committed r143225: <http://trac.webkit.org/changeset/143225> All reviewed patches have been landed. Closing bug. |