Closed Bug 367220 Opened 17 years ago Closed 17 years ago

CSS counter values seem non-deterministic once all counter uses have been removed

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: dbaron)

References

Details

(Keywords: testcase, Whiteboard: [patch])

Attachments

(3 files, 2 obsolete files)

Attached file testcase
The numbers in this testcase keep changing.  The effect is especially noticeable in debug builds.
OS: Mac OS X → All
Hardware: Macintosh → All
Attached patch patch (obsolete) — Splinter Review
The problem was that I forgot to update mSize in DestroyNodesFor.
Attachment #251739 - Flags: superreview?(roc)
Attachment #251739 - Flags: review?(roc)
Blocks: 3247
Status: NEW → ASSIGNED
Whiteboard: [patch]
Comment on attachment 251739 [details] [diff] [review]
patch

er, no, Remove does this, except for the one case where I don't call Remove
Attachment #251739 - Flags: superreview?(roc)
Attachment #251739 - Flags: review?(roc)
Attached patch patch (obsolete) — Splinter Review
OK, this just fixes the one case that was broken.  (So the bug was only present if you had counters, removed *all* of them, and then added more -- size would end up off by one.)

This simplifies code by doing pointer comparison on dangling pointers to see if we just removed the last entry.  A little hairy, but it should be ok.  Remove does some extra fixup we don't need in that case, but I'd rather just call Remove everywhere we do a remove.
Attachment #251739 - Attachment is obsolete: true
Attachment #251740 - Flags: superreview?(roc)
Attachment #251740 - Flags: review?(roc)
Comment on attachment 251740 [details] [diff] [review]
patch

OK, but really, I think you should just do the comparison early on non-dangling pointers and save the result in a boolean.
Attachment #251740 - Flags: superreview?(roc)
Attachment #251740 - Flags: superreview+
Attachment #251740 - Flags: review?(roc)
Attachment #251740 - Flags: review+
Attached patch patchSplinter Review
Good idea.
Attachment #251740 - Attachment is obsolete: true
Checked in to trunk.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Summary: CSS counter values seem non-deterministic → CSS counter values seem non-deterministic once all counter uses have been removed
Flags: in-testsuite?
Attached patch reftestSplinter Review
In old builds, this fails in a different way than the original testcase.  Instead of appearing random, the last number is 4 instead of 2.  In new builds, it passes.
Attachment #257210 - Flags: review?(dbaron)
Comment on attachment 257210 [details] [diff] [review]
reftest

r=dbaron
Attachment #257210 - Flags: review?(dbaron) → review+
reftest checked in.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: