1209710 - [css-grid] Try to simplify Align/JustifySelf by adding a couple of new writing-mode convenience methods

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Mats Palmgren (inactive)]

dholbert notes in bug 1151213 comment #10:
> ...
> Ideally, this whole block of code (up to the marginStart/marginEnd decls)
> could be condensed to:
> 
>   LogicalSide startSide = MakeLogicalSide(aAxis, eLogicalEdgeStart);
>   LogicalSide endSide = MakeLogicalSide(aAxis, eLogicalEdgeEnd);
>   if (MOZ_UNLIKELY(!aSameSide)) {
>     Swap(startSide, endSide);
>   }
>   nscoord marginStart = margin.GetSizeOnSide(wm, startSide);
>   nscoord marginEnd = margin.GetSizeOnSide(wm, endSide);
> 
> ...but unfortunately, the "LogicalMargin::GetSizeOnSide" method used in my
> sample-code here doesn't exist.  It probably should, though. (It should take
> a "LogicalSide" enum.)
> 
> > +                styleMargin.GetBStartUnit(wm) != eStyleUnit_Auto &&
> > +                styleMargin.GetBEndUnit(wm) != eStyleUnit_Auto)
> > +             : (aRS.mStylePosition->ISize(wm).GetUnit() == eStyleUnit_Auto &&
> > +                styleMargin.GetIStartUnit(wm) != eStyleUnit_Auto &&
> > +                styleMargin.GetIEndUnit(wm) != eStyleUnit_Auto)) {
> 
> As above, there's a lot of duplicated logic here to handle the different
> permutations of writing-modes possibilities, which makes this "if" check a
> bit unwieldy & harder to read/maintain.
> 
> I think this can all be abstracted to remove the duplication like so
> (untested):
> 
>   nsStyleCoord styleSize = aAxis == eLogicalAxisBlock ?
>     aRS.mStylePosition->BSize(wm) : aRS.mStylePosition->ISize(wm);
>   if (styleSize.GetUnit() == eStyleUnit_Auto &&
>       styleMargin.GetUnit(startSide) != eStyleUnitAuto &&
>       styleMargin.GetUnit(endSide) != eStyleUnitAuto) {
> 
> (startSide and endSide are as defined in other sample code above.)

Comment 1

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1209710 - Use convenience methods to simplify CSSAlignUtils::AlignJustifySelf().

We already have convenience methods added over the years :)

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0b0850b7413a
Use convenience methods to simplify CSSAlignUtils::AlignJustifySelf(). r=layout-reviewers,jfkthame

Comment 3

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/0b0850b7413a