Re: [evolution-patches] Patch to fix crash when expanding "On this Computer" in mailer



On Mar , 2004-02-24 at 22:18 +0800, Not Zed wrote:
> On Tue, 2004-02-24 at 09:11 -0500, Rodney Dawes wrote:
> 
> > On Tue, 2004-02-24 at 00:43, Not Zed wrote:
> > > On Tue, 2004-02-24 at 00:52 -0500, Rodney Dawes wrote:
> > > 
> > > > This patch fixes the much disputed crashing in the treeview without
> > > 
> > > I'm not sure this patch will affect the issue anyway, since its got
> > > nothign to do with subfolders, empty or otherwise.  Only the top-level
> > > folder being expanded by the code *while* the animation is still running
> > > after the user expanded it.
> > 
> > Hrmm. That doesn't seem right, since the if statement is recursing.
> 
> Which if?

if ((fi->child = scan_dir(store, visited, fi, path, fi->full_name,
flags, ex)))
	fi->flags |= CAMEL_FOLDER_CHILDREN;

In camel-mbox-store.c.

> I tracked the problem down to:
>  user clicks on arrow
>  gtk expands it, and starts an animation timeout to update it
>  the code then sets the state on that folder to expanded
>  gtk clears up some of the animation data, but doesn't remove the
> timeout, and sets the animation to complete
>  the timeout runs later and causes the crash

True. That's what the backtrace was leading to. Since we were getting a
folder with the CAMEL_FOLDER_CHILDREN flag (as we force it upon .sbd
paths), even though the folder had no children (the subdirectory was
empty or only contained hidden files), the expander arrow was appearing,
because we add the "Loading..." node, and we were removing it, before
the animation finished, it exposed the crash and bug in treeview.
However, we shouldn't be expecting folders without children, to have
them, either. So, there are actually two bugs involved in that crash.

> > > > patching gtk+, as we are doing the wrong thing in camel also, when
> > > > loading local folders that don't actually have children, but we force
> > > > the CAMEL_FOLDER_CHILDREN flag even when the sub-directory is empty,
> > > > or only contains files that we want to ignore (those that being with a
> > > > period).
> > > 
> > > Patch looks reasonable, although that flag should not be being set in
> > > the first place really.  There's a conflict with the recursive code vs
> > > the non-recursive scanner.
> > 
> > Here's another patch which just gets rid of the conflicting code then.
> > The same things are still fixed.
> 
> Yeah but you can't do that, since its needed if you scan the folders one
> level at a time, and not recursively, i think.  The original patch was
> fine :)

OK. Committed the original patch then.

-- dobey





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]