Closed Bug 444688 Opened 11 years ago Closed 11 years ago

Scavenge EXCLUDE_IGNORABLE_WHITESPACE frame state bit

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: roc, Assigned: roc)

Details

Attachments

(1 file)

Attached patch fixSplinter Review
I need a frame state bit to track when a frame is being used as the source of background rendering for another element (we need InvalidateInternal to be fast so a property lookup is probably a bad idea). We can replace  EXCLUDE_IGNORABLE_WHITESPACE with an IsFrameOfType check.
Attachment #329009 - Flags: superreview?(dbaron)
Attachment #329009 - Flags: review?(dbaron)
Comment on attachment 329009 [details] [diff] [review]
fix

>-  // If this bit is set, the frame doesn't allow ignorable whitespace as
>-  // children. For example, the whitespace between <table>\n<tr>\n<td>
>-  // will be excluded during the construction of children. 
>-  // The bit is set when the frame is first created and remain
>-  // unchanged during the life-time of the frame.
>-  NS_FRAME_EXCLUDE_IGNORABLE_WHITESPACE =       0x00010000,

Could you move this comment down to eExcludesIgnorableWhitespace rather than deleting it?

>diff --git a/layout/mathml/base/src/nsMathMLmtableFrame.h b/layout/mathml/base/src/nsMathMLmtableFrame.h

I think it's worth changing the other eMathML frame types in this file even though there's some duplication of checking that bit.

>+    return nsSplittableFrame::IsFrameOfType(aFlags &
>+      ~nsIFrame::eExcludesIgnorableWhitespace);

Would you mind parenthesizing the argument to the ~ operator (since that's where new bits get inserted)?  That's what I did for the existing ones.  Or do you think that's ugly?
Comment on attachment 329009 [details] [diff] [review]
fix

r+sr=dbaron with above comments
Attachment #329009 - Flags: superreview?(dbaron)
Attachment #329009 - Flags: superreview+
Attachment #329009 - Flags: review?(dbaron)
Attachment #329009 - Flags: review+
(In reply to comment #1)
> (From update of attachment 329009 [details] [diff] [review])
> >-  // If this bit is set, the frame doesn't allow ignorable whitespace as
> >-  // children. For example, the whitespace between <table>\n<tr>\n<td>
> >-  // will be excluded during the construction of children. 
> >-  // The bit is set when the frame is first created and remain
> >-  // unchanged during the life-time of the frame.
> >-  NS_FRAME_EXCLUDE_IGNORABLE_WHITESPACE =       0x00010000,
> 
> Could you move this comment down to eExcludesIgnorableWhitespace rather than
> deleting it?

Sure.

> >diff --git a/layout/mathml/base/src/nsMathMLmtableFrame.h b/layout/mathml/base/src/nsMathMLmtableFrame.h
> 
> I think it's worth changing the other eMathML frame types in this file even
> though there's some duplication of checking that bit.

OK.

> >+    return nsSplittableFrame::IsFrameOfType(aFlags &
> >+      ~nsIFrame::eExcludesIgnorableWhitespace);
> 
> Would you mind parenthesizing the argument to the ~ operator (since that's
> where new bits get inserted)?  That's what I did for the existing ones.  Or do
> you think that's ugly?

No, that's fine.
Pushed 5dbb5d495a5c.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.