Bug 101108

Created attachment 174764 [details] Initial patch Patch adding CSS support for the 'outside-shape' value of shape-inside. Layout support to follow in a subsequent patch.

Comment on attachment 174764 [details] Initial patch Attachment 174764 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14872382

Created attachment 175009 [details] Fixing windows build failure Changing ExclusionShapeValue to not conditionally compile, as RenderStyle::shapeInside/Outside must return a value regardless of whether CSS Exclusions are enabled.

Comment on attachment 175009 [details] Fixing windows build failure View in context: https://bugs.webkit.org/attachment.cgi?id=175009&action=review I think OutsideExclusionShapeValue does not make any sense. It is adding a new class without any sense IMO. > Source/WebCore/ChangeLog:11 > + auto (null), basic shape, or outside-shape. Eventually there will be a url Well, it is <shape> (a Basic Shape), 'outside-shape' and 'auto'. It should be more clear here. > Source/WebCore/css/CSSValueKeywords.in:917 > // -webkit-wrap-shape this is not called wrap-shape anymore, is it? Can you update this comment please? > Source/WebCore/css/StyleBuilder.cpp:1734 > + setValue(styleResolver->style(), OutsideExclusionShapeValue::create()); There should be an FIXME here, with a link to a bug report. > Source/WebCore/page/animation/CSSPropertyAnimation.cpp:155 > + const BasicShape* fromShape = static_cast<ShapeExclusionShapeValue*>(from)->shape(); > + const BasicShape* toShape = static_cast<ShapeExclusionShapeValue*>(to)->shape(); > + > + if (!fromShape->canBlend(toShape)) > + return to; You may want to blend with outside shape in the future. Please add an FIXME here. > Source/WebCore/rendering/RenderBlock.h:525 > + void updateExclusionShapeInsideInfoAfterStyleChange(const ExclusionShapeValue*, const ExclusionShapeValue* oldWrapShape); Can you rename it to oldExclusionShape? > Source/WebCore/rendering/style/ExclusionShapeValue.h:69 > + : ExclusionShapeValue(SHAPE) > + , m_shape(shape) indentation looks wrong here. > Source/WebCore/rendering/style/ExclusionShapeValue.h:83 > +class OutsideExclusionShapeValue : public ExclusionShapeValue { > +public: > + static PassRefPtr<OutsideExclusionShapeValue> create() > + { > + return adoptRef(new OutsideExclusionShapeValue()); > + } > + > +private: > + OutsideExclusionShapeValue() : ExclusionShapeValue(OUTSIDE) { } > +}; I don't think that you need to create this class at this point. It doesn't have any information. Just use ExclusionShapeValue for everything and remove OutsideExclusionShapeValue and ShapeExclusionShapeValue for now. Then we don't have unnecessary vtables and inheritance.

Created attachment 175074 [details] Incorporating Dirk's feedback

Created attachment 175249 [details] Updating patch

Comment on attachment 175249 [details] Updating patch LGTM. r=me

Comment on attachment 175249 [details] Updating patch Clearing flags on attachment: 175249 Committed r135314: <http://trac.webkit.org/changeset/135314>

All reviewed patches have been landed. Closing bug.