Closed Bug 326033 Opened 14 years ago Closed 12 years ago

debug code in nsTreeBodyFrame::Paint alters class members

Categories

(Core :: XUL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: ajschult784, Assigned: ajschult784)

References

()

Details

Attachments

(2 files, 1 obsolete file)

The debug code in nsTreeBodyFrame::Paint from bug 208093 alters the mRowCount field when it really just wants to check it.
Attached patch patch (obsolete) — Splinter Review
Attachment #210823 - Flags: superreview?(neil)
Attachment #210823 - Flags: review?(neil)
See bug 208093 comment 19.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → INVALID
Comment on attachment 210823 [details] [diff] [review]
patch

Wrong patch anyway - you need to manage your patches better :-P
Attachment #210823 - Flags: superreview?(neil)
Attachment #210823 - Flags: review?(neil)
As a matter of policy debug code should not change program behavior.  It means that debug builds behave differently than release builds.  (Real, non-assertion) bugs are reproducible in one and not the other.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Attached patch the right patchSplinter Review
> Wrong patch anyway - you need to manage your patches better :-P

That, or not attach them a 2am local time.

Also, as in bug 309967, it asserts multiple times because of this, rather than in spite of it.
Attachment #210823 - Attachment is obsolete: true
Comment on attachment 210869 [details] [diff] [review]
the right patch

But if you do this then when someone get the row count wrong you'll get flooded by the paint-time assertions thus making it useless.

(In reply to comment #5)
>Also, as in bug 309967, it asserts multiple times because of this, rather than
>in spite of it.
Bug 309967 was still an abuse of the API, so the asserts were valid.
Flags: blocking1.9a1+
Flags: blocking1.9a1+
Neil - can you take a review of the patch - looks like a straightforward bug fix.. would be great to get it in or close the bug out..
(In reply to comment #6)
>(From update of attachment 210869 [details] [diff] [review])
>But if you do this then when someone get the row count wrong you'll get flooded
>by the paint-time assertions thus making it useless.
You could of course switch to paint-time warnings which wouldn't be so annoying.
Attachment #277707 - Flags: superreview?(bzbarsky)
Attachment #277707 - Flags: review?(Olli.Pettay)
Comment on attachment 277707 [details] [diff] [review]
Change the assertion to a warning to allow flooding

sr=bzbarsky but I personally am OK with the "flooding" thing....  Though I can see how on Windows it would be bad.
Attachment #277707 - Flags: superreview?(bzbarsky) → superreview+
Attachment #277707 - Flags: review?(Olli.Pettay) → review+
Fix checked in.
Status: REOPENED → RESOLVED
Closed: 14 years ago12 years ago
Resolution: --- → FIXED
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: xptoolkit.trees → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.