509956 - Bogus code in nsTextFrame::IsFloatingFirstLetterChild

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Boris Zbarsky [:bzbarsky]]

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.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away until 10/14)]

Attachment: Patch v1.0

Sorry, I missed to check the neil's comment and thank you Boris, you reported this bug.

Comment 2

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

Comment on attachment 394508 [details] [diff] [review]
Patch v1.0

r=dbaron.  (sr is not needed)

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away until 10/14)]

http://hg.mozilla.org/mozilla-central/rev/606a5cf1b1e1

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away until 10/14)]

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.