367442 - [FIX][reflow branch]Possible to end up with bogus mHResize flag in reflow state

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Boris Zbarsky [:bzbarsky]]

nsHTMLReflowState::mComputedWidth should be protected

In bug 364762 I discovered that due to a caller changing mComputedWidth we end up with an incorrect mFlags.mHResize.  We should probably make the member protected and have a getter that just returns it and a setter that resets mFlags as needed.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Like so

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 252268 [details] [diff] [review]
Like so

r+sr=dbaron.

The indentation in nsMathMLmactionFrame is a column off, fwiw.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Actually, that was a tab-indent.... Fixed to use spaces.

And checked in.

Comment 4

[jag (Peter Annema)]

Why not move the assignment inside the test?

  if (mComputedWidth != aComputedWidth) {
    mComputedWidth = aComputedWidth;
    InitResizeFlags(frame->GetPresContext());
  }



-      // XXX fix up bad mComputedWidth for scroll frame
-      kidReflowState.mComputedWidth = PR_MAX(kidReflowState.mComputedWidth, 0);

Why was that no longer needed?

Comment 5

[Boris Zbarsky [:bzbarsky]]

> Why not move the assignment inside the test?

Good idea.  I'll do that.

> Why was that no longer needed?

Because mComputedWidth should never be negative.  It used to be in some buggy cases, but we fixed those a while back.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Address jag's comment

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 261924 [details] [diff] [review]
Address jag's comment

r+sr=dbaron

Comment 8

[Boris Zbarsky [:bzbarsky]]

Checked in that followup patch too.