Review of attachment 9039066 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/ReflowInput.cpp @@ +1679,5 @@ > + left = cb.X(); > + } else { > + right = aReflowInput->ComputedWidth() + > + aReflowInput->ComputedPhysicalBorderPadding().right - > + cb.XMost(); I think this arithmetic is wrong... I think you really want to be finding the delta between the right edge of the padding-box and cb.XMost() (which is a rect in the coordinate system of the padding box[1]), correct? To do that, I think you want the second term here to be aReflowInput->ComputedPhysicalPadding().LeftRight() (Notably: including both sides of the padding, to get the full width of the padding box; and disregarding the border since none of our coordinate systems here are border-box-relative AFAICT.) In particular: if that `cb` rect were snapped to the right edge of the padding box, then we'd *want* to end up with right==0. And in that scenario, I think cb.XMost() would be the full width of the padding box, which means the first two terms have to add up to the full width of the padding box in order to produce a result of 0 from the subtraction. [1] https://searchfox.org/mozilla-central/source/layout/generic/nsGridContainerFrame.cpp#5478-5480 So given that we're only working with padding-rect-relative positions/rects here, it seems like border shouldn't come into play...
Bug 1520584 Comment 10 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Review of attachment 9039066 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/ReflowInput.cpp @@ +1679,5 @@ > + left = cb.X(); > + } else { > + right = aReflowInput->ComputedWidth() + > + aReflowInput->ComputedPhysicalBorderPadding().right - > + cb.XMost(); I think this arithmetic is wrong... I think you really want to be finding the delta between the right edge of the padding-box and cb.XMost() (which is a rect in the coordinate system of the padding box[1]), correct? To do that, I think you want the second term here to be aReflowInput->ComputedPhysicalPadding().LeftRight() (Notably: including both sides of the padding, to get the full width of the padding box; and disregarding the border since none of our coordinate systems here are border-box-relative AFAICT.) In particular: if that `cb` rect were snapped to the right edge of the padding box, then we'd *want* to end up with right==0. And in that scenario, I think cb.XMost() would be the full width of the padding box, which means the first two terms have to add up to the full width of the padding box in order to produce a result of 0 from the subtraction. [1] https://searchfox.org/mozilla-central/source/layout/generic/nsGridContainerFrame.cpp#5478-5480