Ting-Yu Lin [:TYLin] (PDT, UTC-7) Assignee
	Description • 8 years ago

'shape-outside' supports various property values, each of them could be implemented separately. This bug implements the rendering of <shape-box> including the curvature from border-radius.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 6 • 8 years ago
mozreview-review

Comment on attachment 8801008 [details]
Bug 1309467 Part 1 - Rename aBOffset to aBCoord in nsFloatManager::GetFlowArea().

https://reviewboard.mozilla.org/r/85788/#review86420

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 7 • 8 years ago
mozreview-review

Comment on attachment 8801009 [details]
Bug 1309467 Part 2 - Convert BandInfoType to an enum class.

https://reviewboard.mozilla.org/r/85790/#review86422

r=dbaron, although I think I prefer the existing BandInfoType / aInfoType naming, since the parameter is asking about what *type of information* about the bands to return from the function

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 8 • 8 years ago
mozreview-review

Comment on attachment 8801010 [details]
Bug 1309467 Part 3 - Move FloatInfo::mRect construction into FloatInfo's constructor.

https://reviewboard.mozilla.org/r/85792/#review86426

	Ting-Yu Lin [:TYLin] (PDT, UTC-7) Assignee
	Comment 9 • 8 years ago
mozreview-review-reply

Comment on attachment 8801009 [details]
Bug 1309467 Part 2 - Convert BandInfoType to an enum class.

https://reviewboard.mozilla.org/r/85790/#review86422

> I think I prefer the existing BandInfoType / aInfoType naming, since the parameter is asking about what type of information about the bands to return from the function

I pick `BandInfo` because it's shorter. I'm also OK with exisitng naming, so I'll change it back to `BandInfoType` / `aBandInfoType` if you prefer having a `Type` suffix.  Part 4 also add a similar `ShapeInfo`. If the idea is sound, I'll change it to `ShapeInfoType` / `aShapeInfoType` after Part 4 goes through the first review pass.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 10 • 8 years ago

(In reply to Ting-Yu Lin [:TYLin] (UTC+8) from comment #9)
> I pick `BandInfo` because it's shorter. I'm also OK with exisitng naming, so
> I'll change it back to `BandInfoType` / `aBandInfoType` if you prefer having
> a `Type` suffix.  Part 4 also add a similar `ShapeInfo`. If the idea is
> sound, I'll change it to `ShapeInfoType` / `aShapeInfoType` after Part 4
> goes through the first review pass.

I do prefer BandInfoType.  But I think the variable in part 4 should be just ShapeType, not ShapeInfoType or ShapeInfo.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 11 • 8 years ago
mozreview-review

Comment on attachment 8801011 [details]
Bug 1309467 Part 4 - Implement <shape-box> values for shape-outside.

https://reviewboard.mozilla.org/r/85794/#review86428

This seems OK, although it's not clear to me how this design is going to extend to <basic-shape> values.

I admit to not really reviewing the tests.  Let me know if there's something about them you want me to look at.

::: layout/generic/BlockReflowInput.h:116
(Diff revision 1)
>                       nscoord aConsumedBSize = NS_INTRINSICSIZE);
>  
>    /**
>     * Get the available reflow space (the area not occupied by floats)
>     * for the current y coordinate. The available space is relative to
>     * our coordinate system, which is the content box, with (0, 0) in the

Making my comments on the commit message be a comment on the first line:

"Bug 1309467 Part 4 - Implement shape-box values for shape-outside."

Please write "<shape-box>" with the < and >, as it's written in CSS specs.  (MozReview might mangle this with backslashes.)



also "are not" -> "is not"

::: layout/generic/BlockReflowInput.cpp:331
(Diff revision 1)
>  BlockReflowInput::GetFloatAvailableSpaceWithState(
> -                      nscoord aBCoord,
> +  nscoord aBCoord, nsFloatManager::ShapeInfo aShapeInfo,
> -                      nsFloatManager::SavedState *aState) const
> +  nsFloatManager::SavedState *aState) const

please indent this two spaces after the :: (which I suspect is what it was before the nsBlockReflowState -> BlockReflowInput renaming) rather than two spaces after the start of line

::: layout/generic/nsBlockFrame.cpp:3462
(Diff revision 1)
> -          aState.GetFloatAvailableSpaceWithState(aState.mBCoord,
> +          aState.GetFloatAvailableSpaceWithState(
> +            aState.mBCoord, nsFloatManager::ShapeInfo::ShapeOutside,
> -                                                 &floatManagerState);
> +            &floatManagerState);

Please:

 - leave the parameters lined up with the (

 - typedef ShapeInfo into BlockReflowInput from nsFloatManager so that you can just type ShapeInfo::ShapeOutside (so it's short enough) -- and use that elsewhere too

::: layout/generic/nsFloatManager.h:171
(Diff revision 1)
>     */
>    enum class BandInfo { BandFromPoint, WidthWithinHeight };
> +  enum class ShapeInfo { Margin, ShapeOutside };
>    nsFlowAreaRect GetFlowArea(mozilla::WritingMode aWM,
>                               nscoord aBCoord, nscoord aBSize,
> -                             BandInfo aBandInfo,
> +                             BandInfo aBandInfo, ShapeInfo aShapeInfo,

As mentioned in my previous bugzilla comment, please restore BandInfoType, and use ShapeType (throughout)

::: layout/generic/nsFloatManager.h:336
(Diff revision 1)
> +    nscoord ISize(ShapeInfo aShapeInfo) const
> +    {
> +      return aShapeInfo == ShapeInfo::Margin ? ISize() : ShapeBoxRect().width;
> +    }

This doesn't seem to be used.  And given that LineLeft and LineRight appear to have more complexity (I guess so you can add <basic-shape> support, although I'm not quite clear on how that will work), I think you should remove this.

::: layout/generic/nsFloatManager.h:348
(Diff revision 1)
> +    nscoord BSize(ShapeInfo aShapeInfo) const
> +    {
> +      return aShapeInfo == ShapeInfo::Margin ? BSize() : ShapeBoxRect().height;
> +    }
> +    bool IsEmpty(ShapeInfo aShapeInfo) const
> +    {
> +      return aShapeInfo == ShapeInfo::Margin
> +        ? IsEmpty() : ShapeBoxRect().IsEmpty();
> +    }

Same here.  Don't add code that you're not using and that doesn't seem like it's going to extend to the features you want.

::: layout/generic/nsFloatManager.h:370
(Diff revision 1)
> +    // This is the reference box of css shape-outside if specified. The
> +    // coordinate setup is the same as mRect.
> +    mozilla::Maybe<nsRect> mShapeBoxRect;

This comment should mention that it's implementing the <shape-box> values in the spec.

::: layout/generic/nsFloatManager.cpp:595
(Diff revision 1)
> +  MOZ_ASSERT(aShapeInfo == ShapeInfo::ShapeOutside);
> +  const StyleShapeOutside& shapeOutside = mFrame->StyleDisplay()->mShapeOutside;
> +  if (shapeOutside.GetType() == StyleShapeSourceType::None) {

It seems like you already know this information based on whether you have an mShapeBoxRect.  Maybe you should use that instead of getting style data?

(I'm not sure which will extend better to <basic-shape>, though.)

::: layout/reftests/reftest.list:127
(Diff revision 1)
> +# css shape-outside
> +include css-shape-outside/reftest.list

If these tests are justified by the spec, it seems like it would be better to put them in layout/reftests/w3c-css/submitted/css-shapes/, with test metadata (see the other tests in w3c-css/submitted or the CSSWG's test documentation), so that they get shared with other browser vendors.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 12 • 8 years ago
mozreview-review

Comment on attachment 8801012 [details]
Bug 1309467 Part 5 - Make flow area of <shape-box> values respect border-radius.

https://reviewboard.mozilla.org/r/85796/#review88272

Here are some initial comments.  I still need to go through the bulk of the nsFloatManager changes, and also look at the tests.

::: layout/generic/nsFloatManager.h:326
(Diff revision 1)
>      FloatInfo(nsIFrame* aFrame, nscoord aLineLeft, nscoord aBlockStart,
>                const mozilla::LogicalRect& aMarginRect,
>                mozilla::WritingMode aWM, const nsSize& aContainerSize);
>  
>      nscoord LineLeft() const { return mRect.x; }
>      nscoord LineRight() const { return mRect.XMost(); }

Again, in the commit message, use <shape-box> with the < and >.

::: layout/generic/nsFloatManager.h:334
(Diff revision 1)
> -    nscoord LineLeft(ShapeInfo aShapeInfo) const;
> -    nscoord LineRight(ShapeInfo aShapeInfo) const;
> +    nscoord LineLeft(ShapeInfo aShapeInfo, const nscoord aBStart,
> +                     const nscoord aBEnd) const;
> +    nscoord LineRight(ShapeInfo aShapeInfo, const nscoord aBStart,
> +                      const nscoord aBEnd) const;

This needs documentation about what aBStart and aBEnd mean.  Presumably they're the starting and ending coordinates of a band, and LineLeft and LineRight give the largest line-left/line-right extent of the float within that band?

::: layout/generic/nsFrame.cpp:1382
(Diff revision 1)
> +nsIFrame::GetMarginBoxBorderRadii(nscoord aRadii[8]) const
> +{
> +  if (!GetBorderRadii(aRadii)) {
> +    return false;
> +  }
> +  OutsetBorderRadii(aRadii, GetUsedMargin());

This is depending on the previously-unused function OutsetBorderRadii.

That function also appears not to be quite correct.

In particular, when dealing with margins (unlike border and padding), you need to consider the possibility that margins are negative.

I think OutsetBorderRadii should deal with the possibility of offset being negative by adding a std::max(0, ...) when it is adding offset.  In other words, I think it probably should allow a negative margin to reduce the radius, but not below zero.

However, this behavior should be specified in a spec somewhere.  If it isn't in a spec somewhere, could you raise an issue on a relevant spec (shapes, if there's not something else that it's referencing).

::: layout/generic/nsIFrame.h:1178
(Diff revision 1)
>    virtual bool GetBorderRadii(const nsSize& aFrameSize,
>                                const nsSize& aBorderArea,
>                                Sides aSkipSides,
>                                nscoord aRadii[8]) const;
>    bool GetBorderRadii(nscoord aRadii[8]) const;
> -
> +  bool GetMarginBoxBorderRadii(nscoord aRadii[8]) const;

Please document that GetMarginBoxBorderRadii and GetShapeBoxBorderRadii only make sense for things that establish block formatting contexts, because they don't participate in margin-collapsing.

::: layout/reftests/css-shape-outside/reftest.list:12
(Diff revision 1)
> +
> +# shape-box with border-radius

again, it would be good to put these in layout/reftests/w3c-css/submitted/, with appropriate metadata

	Ting-Yu Lin [:TYLin] (PDT, UTC-7) Assignee
	Comment 18 • 8 years ago
mozreview-review-reply

Comment on attachment 8801011 [details]
Bug 1309467 Part 4 - Implement <shape-box> values for shape-outside.

https://reviewboard.mozilla.org/r/85794/#review86428

I move those reftest to layout/reftests/w3c-css/submitted/shapes1/ and added meta for each test. I'll appreciate if you could skim throught those meta data and see if anything is wrong to block them from upstream.

> Please:
> 
>  - leave the parameters lined up with the (
> 
>  - typedef ShapeInfo into BlockReflowInput from nsFloatManager so that you can just type ShapeInfo::ShapeOutside (so it's short enough) -- and use that elsewhere too

Good idea to typedef to make `ShapeType` shorter, but I need to make alias in both nsBlockFramec.cpp and `BlockReflowInput` to achieve that. And I feel inconsistent without alias `BandInfoType` in `BlockReflowInput` at some call sites, so I'd like to alias `BandInfoType` as well.

> It seems like you already know this information based on whether you have an mShapeBoxRect.  Maybe you should use that instead of getting style data?
> 
> (I'm not sure which will extend better to <basic-shape>, though.)

Yes. We should have an mShapeBoxRect if and only if we have `shape-outside` style specified (even if the <shape-box> for basic-shape is unspecified), but we'll still need to get style data to implmenting basic-shape anyway. Let's use style data for now. Things will get clearer once I have some patches for basic-shapes.

> If these tests are justified by the spec, it seems like it would be better to put them in layout/reftests/w3c-css/submitted/css-shapes/, with test metadata (see the other tests in w3c-css/submitted or the CSSWG's test documentation), so that they get shared with other browser vendors.

OK. Move the tests to layout/reftests/w3c-css/submitted/ and add the metadata.

	Ting-Yu Lin [:TYLin] (PDT, UTC-7) Assignee
	Comment 19 • 8 years ago
mozreview-review-reply

Comment on attachment 8801012 [details]
Bug 1309467 Part 5 - Make flow area of <shape-box> values respect border-radius.

https://reviewboard.mozilla.org/r/85796/#review88272

> This is depending on the previously-unused function OutsetBorderRadii.
> 
> That function also appears not to be quite correct.
> 
> In particular, when dealing with margins (unlike border and padding), you need to consider the possibility that margins are negative.
> 
> I think OutsetBorderRadii should deal with the possibility of offset being negative by adding a std::max(0, ...) when it is adding offset.  In other words, I think it probably should allow a negative margin to reduce the radius, but not below zero.
> 
> However, this behavior should be specified in a spec somewhere.  If it isn't in a spec somewhere, could you raise an issue on a relevant spec (shapes, if there's not something else that it's referencing).

I think the spec doesn't consider negative margin properly. I filed https://github.com/w3c/csswg-drafts/issues/675. Also the spec requires the margin to be adjusted when the ratio of border-radius/margin is less than 1, but the equation doesn't make sense with negative margin. So I ignore it until the spec has some clarification. 

I've modified OutsetBorderRadii() to allow it deal with negative margin.

	Ting-Yu Lin [:TYLin] (PDT, UTC-7) Assignee
	Comment 21 • 8 years ago

David, the current patchset addressed all the previous review comments. Please help review it. Thank you.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 22 • 8 years ago
mozreview-review

Comment on attachment 8801012 [details]
Bug 1309467 Part 5 - Make flow area of <shape-box> values respect border-radius.

https://reviewboard.mozilla.org/r/85796/#review91828

r=dbaron, although I didn't look at the tests very closely

I'd prefer that fixing the logical directions issue be a separate patch, but it should be filed so you can put it in the FIXME comment, and it shoul block bug 1098939 and bug 1309467

::: layout/generic/nsFloatManager.h:336
(Diff revision 3)
> +    // LineLeft() returns the smallest line-left extent and LineRight()
> +    // returns the largest line-right extent within the given band;

I think smallest and largest here are backwards, although it might make more sense to say innermost.

::: layout/generic/nsFloatManager.h:352
(Diff revision 3)
>      nscoord BEnd(ShapeType aShapeType) const
>      {
>        return aShapeType == ShapeType::Margin ? BEnd() : ShapeBoxRect().YMost();
>      }
>  
> +    // Compute the minimal x-axis difference between the bounding shape box

minimal -> minimum

::: layout/generic/nsFloatManager.h:353
(Diff revision 3)
> +    // and its rounded corner within the constraint of the given band's
> +    // y-axis. This is used as a helper function to compute the LineRight()

constraint of the given band's y-axis -> within the given band (y-axis region)

::: layout/generic/nsFloatManager.h:357
(Diff revision 3)
> +    // Compute the minimal x-axis difference between the bounding shape box
> +    // and its rounded corner within the constraint of the given band's
> +    // y-axis. This is used as a helper function to compute the LineRight()
> +    // and LineLeft(). See the picture in the implementation for an example.
> +    //
> +    // Returns the x-axis diff, or 0 if there's no rounded corner between

between -> within

::: layout/generic/nsFloatManager.cpp:216
(Diff revision 3)
> +      // When the blockEnd is nscoord_MAX, we assume the band's block size is
> +      // unknown. We treat it as a 0-height band by passing blockStart to
> +      // LineRight() or LineLeft() to satisfy the invariant that the flow space
> +      // doesn't grow in a WidthWithinHeight call when the height of the band is
> +      // known.
> +      const nscoord bandBlockEnd =
> +        blockEnd == nscoord_MAX ? blockStart : blockEnd;

I think it makes more sense to make the condition be
aInfoType == BAND_FROM_POINT ? blockStart : blockEnd;
and then have the comment say that for BAND_FROM_POINT, we're only intended to consider a point along the y axis rather than a band.

::: layout/generic/nsFloatManager.cpp:613
(Diff revision 3)
>    if (shapeOutside.GetType() == StyleShapeSourceType::None) {
>      return LineLeft();
>    }
>  
>    if (shapeOutside.GetType() == StyleShapeSourceType::Box) {
> +    nscoord radii[8] = {0};

don't 0-initialize

::: layout/generic/nsFloatManager.cpp:621
(Diff revision 3)
> +        ShapeBoxRect().y, ShapeBoxRect().YMost(),
> +        radii[NS_CORNER_TOP_LEFT_X], radii[NS_CORNER_TOP_LEFT_Y],
> +        radii[NS_CORNER_BOTTOM_LEFT_X], radii[NS_CORNER_BOTTOM_LEFT_Y],
> +        aBStart, aBEnd);

this (and the same in LineRight) don't look like they'll do the right thing for non-ltr direction/writing-mode.  It would be good to both fix that and add tests, although I'm ok with (in this patch) doing a FIXME comment pointing to a bug that blocks the bug on enabling this feature, and then fixing it in a separate bug.

::: layout/generic/nsFloatManager.cpp:649
(Diff revision 3)
>    if (shapeOutside.GetType() == StyleShapeSourceType::None) {
>      return LineRight();
>    }
>  
>    if (shapeOutside.GetType() == StyleShapeSourceType::Box) {
> +    nscoord radii[8] = {0};

don't 0-initialize

::: layout/generic/nsFloatManager.cpp:672
(Diff revision 3)
> +  const nscoord aTopCornerRadiusX, const nscoord aTopCornerRadiusY,
> +  const nscoord aBottomCornerRadiusX, const nscoord aBottomCornerRadiusY,

When you fix the issue about logical coordinates, these should probably have logical names.

::: layout/generic/nsFloatManager.cpp:717
(Diff revision 3)
> +
> +  nscoord xDiff = 0;
> +
> +  if (aBandYMost >= aShapeBoxY &&
> +      aBandYMost <= aShapeBoxY + aTopCornerRadiusY) {
> +    // The band intersects the top corner.

Maybe say intersects *only* the top corner, and then comment that if it intersects both corners we don't need to enter either branch because we know the correct XDiff is 0.

::: layout/generic/nsFloatManager.cpp:739
(Diff revision 3)
> +/* static */ nscoord
> +nsFloatManager::FloatInfo::XInterceptAtY(const nscoord aY,
> +                                         const nscoord aRadiusX,
> +                                         const nscoord aRadiusY)
> +{
> +  // Solve x for the ellipse equation (x/radiusX)^2 + (y/radiusY)^2 = 1.

"Solve x for" -> "Solve for x in"

::: layout/generic/nsFloatManager.cpp:741
(Diff revision 3)
> +                                         const nscoord aRadiusX,
> +                                         const nscoord aRadiusY)
> +{
> +  // Solve x for the ellipse equation (x/radiusX)^2 + (y/radiusY)^2 = 1.
> +  MOZ_ASSERT(aRadiusY > 0);
> +  return aRadiusX * std::sqrt(1 - (aY*aY) / static_cast<float>(aRadiusY*aRadiusY));

Please use construction style cast "float()" rather than static_cast.  Also please cast to double() given that it's going to convert to double anyway to call sqrt... unless you'd rather call sqrtf.

::: layout/generic/nsFrame.cpp:1341
(Diff revision 3)
> -    if (aRadii[hc1] > 0)
> -      aRadii[hc1] += offset;
> -    if (aRadii[hc2] > 0)
> -      aRadii[hc2] += offset;
> +    if (aRadii[hc1] > 0) {
> +      aRadii[hc1] = std::max(0, aRadii[hc1] + offset);
> +    }
> +    if (aRadii[hc2] > 0) {
> +      aRadii[hc2] = std::max(0, aRadii[hc2] + offset);
> +    }

I think you should implement the cubic formula in the spec here, but only for offset > 0, as I suggested in https://github.com/w3c/csswg-drafts/issues/675#issuecomment-259605711

::: layout/reftests/w3c-css/submitted/shapes1/shape-outside-border-box-border-radius-001.html:8
(Diff revision 3)
> +   - http://creativecommons.org/publicdomain/zero/1.0/ -->
> +
> +<html>
> +  <meta charset="utf-8">
> +  <title>CSS Shape Test: float left, border-box, border-radius</title>
> +  <link rel="author" title="Ting-Yu Lin" href="mailto:tlin@mozilla.com">

Could you add a second author line to all of these that says "Mozilla"?  (See many of the other tests in w3c-css/submitted/.)

::: layout/reftests/w3c-css/submitted/shapes1/shape-outside-border-box-border-radius-001.html:10
(Diff revision 3)
> +<html>
> +  <meta charset="utf-8">
> +  <title>CSS Shape Test: float left, border-box, border-radius</title>
> +  <link rel="author" title="Ting-Yu Lin" href="mailto:tlin@mozilla.com">
> +  <link rel="help" href="https://drafts.csswg.org/css-shapes/#shapes-from-box-values">
> +  <link rel="match" href="shape-outside-border-box-border-radius-001-ref.html">

Please run layout/reftests/w3c-css/submitted/check-for-references.sh to check all the rel="match" lines if you haven't done so already.

	Ting-Yu Lin [:TYLin] (PDT, UTC-7) Assignee
	Comment 28 • 8 years ago
mozreview-review-reply

Comment on attachment 8801012 [details]
Bug 1309467 Part 5 - Make flow area of <shape-box> values respect border-radius.

https://reviewboard.mozilla.org/r/85796/#review91828

Rebased on top of bug 1291110, and mark those tests pass in reftest.list.

> I think it makes more sense to make the condition be
> aInfoType == BAND_FROM_POINT ? blockStart : blockEnd;
> and then have the comment say that for BAND_FROM_POINT, we're only intended to consider a point along the y axis rather than a band.

Yes. It's reasonable to use BandFromPoint. Comment also updated.

> don't 0-initialize

I guess it's because `radii` is going to be initialized in `GetShapeBoxBorderRadii(radii)` anyway.

> this (and the same in LineRight) don't look like they'll do the right thing for non-ltr direction/writing-mode.  It would be good to both fix that and add tests, although I'm ok with (in this patch) doing a FIXME comment pointing to a bug that blocks the bug on enabling this feature, and then fixing it in a separate bug.

Both mRect and mShapeBoxRect are stored in nsRect although they're actually a logical rect. That's why I use physical coordinate names in the first place. However, I haven't consider non-ltr direction and writing mode yet. And you're right we need tests. Filed bug 1316549.

> Please use construction style cast "float()" rather than static_cast.  Also please cast to double() given that it's going to convert to double anyway to call sqrt... unless you'd rather call sqrtf.

OK. I've changed construction style cast and use double. However, unlike C, C++'s std::sqrt() is overloaded with all the numeric types, so either float and double should be fine. http://en.cppreference.com/w/c/numeric/math/sqrt

> I think you should implement the cubic formula in the spec here, but only for offset > 0, as I suggested in https://github.com/w3c/csswg-drafts/issues/675#issuecomment-259605711

OK. Implemented.

> Could you add a second author line to all of these that says "Mozilla"?  (See many of the other tests in w3c-css/submitted/.)

Sure. Added "Mozilla" as the second author for test cases in Part 4 as well.

> Please run layout/reftests/w3c-css/submitted/check-for-references.sh to check all the rel="match" lines if you haven't done so already.

Thanks for the tip. Luckly, I found that I get all the match lines correct after running the script.

	Pulsebot
	Comment 29 • 8 years ago

Pushed by tlin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3c12d8818326
Part 1 - Rename aBOffset to aBCoord in nsFloatManager::GetFlowArea(). r=dbaron
https://hg.mozilla.org/integration/autoland/rev/9ac5a902d519
Part 2 - Convert BandInfoType to an enum class. r=dbaron
https://hg.mozilla.org/integration/autoland/rev/4ff1f76b0498
Part 3 - Move FloatInfo::mRect construction into FloatInfo's constructor. r=dbaron
https://hg.mozilla.org/integration/autoland/rev/892b614625fe
Part 4 - Implement <shape-box> values for shape-outside. r=dbaron
https://hg.mozilla.org/integration/autoland/rev/979937f3349e
Part 5 - Make flow area of <shape-box> values respect border-radius. r=dbaron

	Carsten Book [:Tomcat]
	Comment 30 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/3c12d8818326
https://hg.mozilla.org/mozilla-central/rev/9ac5a902d519
https://hg.mozilla.org/mozilla-central/rev/4ff1f76b0498
https://hg.mozilla.org/mozilla-central/rev/892b614625fe
https://hg.mozilla.org/mozilla-central/rev/979937f3349e

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 31 • 8 years ago

All of the tests gave the following error when I did the import into the CSS WG test repository:

remote: vendor-imports/mozilla/mozilla-central-reftests/shapes1/shape-outside-border-box-001.html status changed to 'Needs Work' due to error:
remote: Linked to unversioned specification URL https://drafts.csswg.org/css-shapes/#shapes-from-box-values. Use https://www.w3.org/TR/css-shapes-1/... or https://drafts.csswg.org/css-shapes-1/... instead.

Would you mind fixing this?  (Just adding the "-1" is fine -- no need to ask for review of that patch.)

	Pulsebot
	Comment 32 • 8 years ago

Pushed by tlin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e8665b1b664c
followup - Fix link to versioned css-shapes spec. r=me

	Ting-Yu Lin [:TYLin] (PDT, UTC-7) Assignee
	Comment 33 • 8 years ago

(In reply to David Baron :dbaron: ⌚️UTC-8 from comment #31)
> Would you mind fixing this?  (Just adding the "-1" is fine -- no need to ask
> for review of that patch.)

Fixed by adding "-1" to the url. Thank you.

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 34 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/e8665b1b664c

	Sebastian Zartner [:sebo]
	Comment 35 • 7 years ago

Added a note about this to https://developer.mozilla.org/en-US/docs/Web/CSS/shape-outside#Browser_compatibility.

Sebastian

	Sebastian Zartner [:sebo]
	Comment 36 • 7 years ago

I've also added a note to https://developer.mozilla.org/en-US/Firefox/Experimental_features#shape-outside, now.

Sebastian