Closed
Bug 509956
Opened 15 years ago
Closed 15 years ago
Bogus code in nsTextFrame::IsFloatingFirstLetterChild
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a1
People
(Reporter: bzbarsky, Assigned: masayuki)
References
Details
Attachments
(1 file)
|
629 bytes,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
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.
|
Assignee | |
Updated•15 years ago
|
Status: NEW → ASSIGNED
|
|
Assignee | |
Comment 1•15 years ago
|
||
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+
|
|
Assignee | |
Updated•15 years ago
|
Keywords: checkin-needed
|
|
Assignee | |
Comment 3•15 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
|
|
Assignee | |
Comment 4•15 years ago
|
||
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.
Description
•