[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kmail-devel
Subject:    Bug#30475: crash when typing f while kmail starts
From:       Don Sanders <sanders () kde ! org>
Date:       2001-08-20 12:06:18
[Download RAW message or body]

On Sunday 19 August 2001 17:24, Ingo Klöcker wrote:
> On Sonntag, 19. August 2001 15:24, Don Sanders wrote:
> > On Sunday 19 August 2001 11:54, Ingo Klöcker wrote:
> > > On Sonntag, 19. August 2001 00:29, Ingo Klöcker wrote:
> > > > No. But I found the cause of the crash. KMail
> > > > crashes in the following line
> > > >
> > > >   win->setCharset(msg->codec()->name(), TRUE);
> > > >
> > > > near the end of KMHeaders::forwardMsg(). The reason
> > > > for the crash is msg->codec() == NULL.
> > > > I guess the codec of the current message still
> > > > hasn't been initialized when the forward action is
> > > > processed.
> > > >
> > > > The attached simple patch will prevent this crash.
> > > >
> > > > This fix will have to be applied to all message
> > > > related actions, e.g. KMHeaders::redirectMsg(),
> > > > KMHeaders::replyToMsg(), etc., because they all
> > > > contain the above line. (I haven't tried if they
> > > > all crash KMail.)
> > >
> > > KMail does indeed also crash if you hold down 'r' or
> > > 'e', etc., while KMail starts.
> > >
> > > The attached patch fixes this problem in all cases.
> >
> > And you think it's best just to return? Rather than to
> > say
> >
> > +if (msg->codec())
> >    win->setCharset(msg->codec()->name(), TRUE);
>
> Well, the charset has to be set to some reasonable value.
> So only setting the charset if msg->codec() != 0 isn't a
> good solution. A slightly better solution would be
>
>  +if (msg->codec())
>     win->setCharset(msg->codec()->name(), TRUE);
>  +else
>  +  win->setCharset("Unicode", TRUE);
>
> But please keep in mind that this problem only occurs if
> the user holds one of the message related keys pressed
> while KMail is started.
>
> > I wonder what the root of the problem is, well if you
> > have a fix please commit.
>
> The root of the problem seems to be that
> msg->setCodec(...) is only called in
> KMReaderWin::setMsg() and in KMReaderWin::parseMsg().
> This means that codec is NULL before any of these two
> functions is executed. 

Oh! Great detective work, that's not obvious at all.

> And this seems to be the case
> right after the start of KMail because the first actions
> (e.g. the forward action) are already processed before
> the message is displayed in the reader pane.
>
> My fix only cures the symptoms, i.e. the processing of
> the message related actions will be aborted until the
> message is shown correctly in the reader pane (resp.
> until the codec is set correctly).
>
> The correct fix would be to either move the setting of
> the codec from KMReaderWin to KMMessage or to copy the
> code which chooses the correct codec from
> KMReaderWin::parseMsg() to KMHeaders so that the codec
> can be set to a reasonable value if it's still unset.
> BTW, this will be important if we ever make it possible
> to turn of the reader pane because then the codec won't
> be set by the corresponding code in KMReaderWin.
>
> The following is the code which is responsible for the
> automatic selection of the codec (cf.
> KMReaderWin::parseMsg()):
>
>     mCodec = 0;
>     QCString encoding;
>     if (type.find("text/") != -1)
>       encoding = mMsg->charset();
>     else if (type.find("multipart/") != -1) {
>       if (mMsg->numBodyParts() > 0) {
>         KMMessagePart msgPart;
>         mMsg->bodyPart(0, &msgPart);
>         encoding = msgPart.charset();
>       }
>     }
>     if (encoding.isEmpty())
>       encoding = "iso8859-1";
>     mCodec = KMMsgBase::codecForName(encoding);
>
> IMHO this code could be moved to the initialization of a
> KMMessage object, i.e. maybe to KMMessage::KMMessage().
> I'm not completely sure if this is the right place.
>
> What do you think about this?

Hmm. How the KMReaderWin code sets the codec of the current 
message is unnecessarily hard to follow.

Yes this sounds like a great idea that should make the code 
easier to understand and safer. Please go ahead and make it.

I can't see any problems being introduced by that change.

(Aside:

That delayed showing of messages in the readerwindow code I 
introduced has been the cause of a great number of 
difficult to understand bugs. I only did it so that if the 
user holds down say the previous message key KMail can 
quickly iterate through messages without having to show 
each one in the readerwindow.

But that nice ergonomic touch has pretty much been broken 
by the KMMainWin::updateMessageMenu code being slow and now 
being called everytime a message is chosen.)

Don.
_______________________________________________
Kmail Developers mailing list
Kmail@mail.kde.org
http://mail.kde.org/mailman/listinfo/kmail

[prev in list] [next in list] [prev in thread] [next in thread]