Closed
Bug 240245
Opened 21 years ago
Closed 15 years ago
nsTableFrame::CreateAnonymousColFrames does bizarre things
Categories
(Core :: Layout: Tables, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: bzbarsky, Assigned: bernd_mozilla)
References
Details
Attachments
(1 file)
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
See bug 233463 comment 3 and bug 233463 comment 19. Basically, this code looks
like it's creating a bogus looping linked list.
<reporter e-mail> - <buildID> is too old a build to report bugs against
now. The problem you are reporting may well have been fixed already.
Could you please download a recent nightly build from
<ftp://ftp.mozilla.org/pub/mozilla/nightly/>, and then let us know if you
still see this problem?
Thanks,
<sign>
yeah and and a reduced testcase would be fine, and please avoid referring to
the random rants like in bug 233463 comment 19.
and don't forget to read the forums or you will end as coder but not as a
developer :-).
Reporter | ||
Comment 2•21 years ago
|
||
Bernd, am I missing something? If that's a request to create a testcase, I can
try to do that, I guess, but I'm not sure what the rest of that comment is about...
a) did the behaviuor change with the patch for bug 238999?
b) is it still an issue?
c) what should I do with that, how can I test it?
d) is there a testcase?
Reporter | ||
Comment 4•21 years ago
|
||
> a) did the behaviuor change with the patch for bug 238999?
Unlikely, since you didn't change the code in question. See
> b) is it still an issue?
I'll try to check; I no longer have the patch that triggered those asserts in my
tree, so that may be hard...
> c) what should I do with that, how can I test it?
I'll work on this.
> d) is there a testcase?
nytimes.com, as bug 233463 comment 3 says.
Reporter | ||
Comment 5•21 years ago
|
||
OK, so there are in fact no loops involved on the nytimes site (which explains
why we don't hang)
The reason there are not is that this function has only one caller, and that
caller always (well, unless aDoAppend is false...) passes
aColGroupFrame.GetChildList().LastChild() as aPrevFrameIn. So aPrevFrameIn ==
lastColFrame at the start of this function in all cases I've been able to find,
which leads to "correct" behavior.
So I think we can downgrade this code from "probably causes hangs in some cases"
to "just doing bizarre things".
We could probably eliminate some arguments to these functions, simplify the code
to just use nsFrameList for the frame list manipulations (and in the process
ensure that no one-line changes anywhere cause hangs), etc.
action items:
1. rename the function so that makes clear this is a append
2. remove all arguments that are not a number of cols to append
3. look up the last col from mColFrames if it is anonmyous cell based append the cols just behind.
4. if not a create a cellbased anonymous colgroup and append the specified number of cols there.
Assignee: nobody → bernd_mozilla
Attachment #387825 -
Flags: review?(bzbarsky)
Reporter | ||
Updated•15 years ago
|
Attachment #387825 -
Flags: review?(bzbarsky) → review+
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•