Boris Zbarsky [:bzbarsky] Reporter
	Description • 21 years ago

See bug 233463 comment 3 and bug 233463 comment 19.  Basically, this code looks
like it's creating a bogus looping linked list.

	Bernd Assignee
	Comment 1 • 20 years ago

<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 :-).

	Boris Zbarsky [:bzbarsky] Reporter
	Comment 2 • 20 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...

	Bernd Assignee
	Comment 3 • 20 years ago

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?

	Boris Zbarsky [:bzbarsky] Reporter
	Comment 4 • 20 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.

	Boris Zbarsky [:bzbarsky] Reporter
	Comment 5 • 20 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.

	Bernd Assignee
	Comment 6 • 15 years ago

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.

	Bernd Assignee
	Comment 8 • 15 years ago

http://hg.mozilla.org/mozilla-central/rev/04bef1a2e0c7