Bogus code in nsTextFrame::IsFloatingFirstLetterChild

RESOLVED FIXED in mozilla1.9.3a1

Status

()

defect
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: bzbarsky, Assigned: masayuki)

Tracking

Trunk
mozilla1.9.3a1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

This part of nsTextFrame:IsFloatingFirstLetterChild:

  if (!GetStateBits() & TEXT_FIRST_LETTER)
    return PR_FALSE;

does absolutely nothing, since ! has higher precedence than &.  Therefore we're doing either 0 & TEXT_FIRST_LETTER or 1 & TEXT_FIRST_LETTER, both of which are 0.  I assume the compiler just optimizes this whole block away.

If this block is needed for performance reasons, we need to fix the parentheses.  If it's not needed, it should be removed so as to not confuse people.

Note that this was pointed out almost a year ago in bug 426772 comment 55.
Posted patch Patch v1.0Splinter Review
Sorry, I missed to check the neil's comment and thank you Boris, you reported this bug.
Attachment #394508 - Flags: superreview?(roc)
Attachment #394508 - Flags: review?(roc)
Comment on attachment 394508 [details] [diff] [review]
Patch v1.0

r=dbaron.  (sr is not needed)
Attachment #394508 - Flags: superreview?(roc)
Attachment #394508 - Flags: review?(roc)
Attachment #394508 - Flags: review+
I'm not sure this patch should be also landed to 1.9.2 branch. The IsFloatingFirstLetterChild is called from nsTextFrame::Reflow. So, that is called many times. But I'm not sure the impact for the performance.
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9.3a1
You need to log in before you can comment on or make changes to this bug.