We have LayoutUnit, LayoutPoint, LayoutSize, LayoutRect, and LayoutBox. Spot the odd one out. Also LayoutBox is an enum, not a "box" in the CSS sense. It needs a better name.
Maybe "CSSBoxType"
(In reply to comment #1) > Maybe "CSSBoxType" +1
I'm fine with CSSBoxType. If you change it, don't forget to change LayoutBoxExtent.h too.
(In reply to comment #3) > I'm fine with CSSBoxType. If you change it, don't forget to change LayoutBoxExtent.h too. Hrm. Does the name CSSBoxExtent actually make sense? I'm not sure.
CSSBoxModelType and CSSBoxModelExtent?
(In reply to comment #5) > CSSBoxModelType and CSSBoxModelExtent? LayoutBoxExtent is more related to LayoutUnit than LayoutBox, so I don't think it needs to be renamed.
Created attachment 227088 [details] Patch
Comment on attachment 227088 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227088&action=review > Source/WebCore/rendering/shapes/Shape.h:72 > - static PassOwnPtr<Shape> createLayoutBoxShape(const RoundedRect&, WritingMode, Length margin, Length padding); > + static PassOwnPtr<Shape> createBoxShape(const RoundedRect&, WritingMode, Length margin, Length padding); Not sure that "box shape" makes sense here, since you're not passing a CSSBoxType parameter. > Source/WebCore/rendering/style/RenderStyleConstants.h:559 > +enum CSSBoxType { BoxMissing = 0, MarginBox, BorderBox, PaddingBox, ContentBox, Fill, Stroke, ViewBox }; This patch makes me wonder if we should call this a CSSReferenceBox
(In reply to comment #8) > (From update of attachment 227088 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=227088&action=review > > > Source/WebCore/rendering/shapes/Shape.h:72 > > - static PassOwnPtr<Shape> createLayoutBoxShape(const RoundedRect&, WritingMode, Length margin, Length padding); > > + static PassOwnPtr<Shape> createBoxShape(const RoundedRect&, WritingMode, Length margin, Length padding); > > Not sure that "box shape" makes sense here, since you're not passing a CSSBoxType parameter. The problem is that this never should have been called "LayoutBoxShape" in the first place, as it creates a BoxShape object. Also, since the Shape classes don't deal with renderers themselves, it expects the caller (ShapeInfo::computedShape) to look at the BoxTime and create a ReoundedRect with the proper dimensions. > > > Source/WebCore/rendering/style/RenderStyleConstants.h:559 > > +enum CSSBoxType { BoxMissing = 0, MarginBox, BorderBox, PaddingBox, ContentBox, Fill, Stroke, ViewBox }; > > This patch makes me wonder if we should call this a CSSReferenceBox Strictly speaking, it is only a Reference Box when it is specified with another shape. If the box is specified by itself, then the spec calls it a Box Shape, because in that case, it is the box that specifies the shape. I tried to change the names from layoutBox to cssBox when it isa a box shape, and to referenceBox when it applies to another shape, but there were a couple of places where that wouldn't have been possible without refactoring.
Comment on attachment 227088 [details] Patch Clearing flags on attachment: 227088 Committed r165843: <http://trac.webkit.org/changeset/165843>
All reviewed patches have been landed. Closing bug.