502681 - Crash if I select all after opening a message in a new tab without changing the focus [@ nsMsgDBView::DoCommand(int)]

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[Daniel]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.1pre) Gecko/20090706 Shredder/3.0b3pre

When I open a message in its own tab and Select All (either from the menu or by hitting cmd-a) Shredder crashes immediately. It doesn't happen in its own window or in the message pane, just when a message is open in a tab.

Additional note, it doesn't crash if I click in the message body before selecting all, only if I select all when whatever is focused after creating a new tab is focused. I can't tell where the focus is, but if I select the topmost element of the header (From: on an incoming message), hit shift-tab to move the focus back one, and then select all it crashes.

This is on an IMAP account.


Reproducible: Always

Steps to Reproduce:
1.Double click message to open in a new tab
2.Hit Cmd-A or Edit-Select-All
Actual Results:  
Crash!

Expected Results:  
Select all message body text

When I open in a new window instead of a tab, it works correctly. 

I've submitted a bunch of talkbacks, but I can't figure out how to tell what the Talkback crash ID is.  The instructions (http://kb.mozillazine.org/Quality_Feedback_Agent) don't seem accurate.

Comment 1

[Mark Banner (:standard8)]

(In reply to comment #0)
> I've submitted a bunch of talkbacks, but I can't figure out how to tell what
> the Talkback crash ID is.  The instructions
> (http://kb.mozillazine.org/Quality_Feedback_Agent) don't seem accurate.

For the Thunderbird 3 builds you want:

http://kb.mozillazine.org/Breakpad

Comment 2

[Daniel]

Thanks for the link.

One of the talkback ids is
cbe4caaa-f97e-47fa-9d84-7bdcb2090706

and the crash is in nsMsgDBView::DoCommand(int)

Comment 3

[Phil Ringnalda (:philor)]

Looks like your missing step 0 is probably "toggle mail.operate_on_msgs_in_collapsed_threads to false" since where focus is when you open a message in a tab is "still in the threadpane of the now-hidden 3pane tab" and with that pref at the default, true, I just get the message tab turned into a summary page for all the messages in the threadpane, which is ugly and wrong and baffling, but a bit nicer than the crash when that pref is false.

Comment 4

[Mark Banner (:standard8)]

Well I can reproduce it with mail.operate_on_msgs_in_collapsed_threads as default (true) using the steps in comment 0. Though since you're playing with that pref maybe you're trying this on a thread rather than a single message.

Comment 5

[Mark Banner (:standard8)]

Attachment: Proposed improvement

This doesn't fully fix it the way I would like, but at least stops the crash. The crash is caused because the focus is on the tabmail element, so it picks up the default window controller and passes it the cmd_selectAll command, our DefaultController code then takes this and tries to select all in the thread pane. This then messes around with things behind the scenes, decides to clear the display and ends up closing the tab which nulls out mTree before we get to it.

So I've added a check for a null tree in nsMsgDBView that fixes the crash. However the tab would still be closed, so I've put in an additional check that say if we're on a message tab, don't do the select all.

This has the effect that if the tabmail element or the message pane are focussed, then nothing will happen when attempting to select all. If the messagepane browser element is selected, then we'll select all as normal.

From what Firefox does, we should be able to do this - I think they don't allow focus on their tabs or something, but we'd need to do more digging for that I think.

For b3 I think this fix is sufficient, if we accept it then I'll file a follow-up bug to look at the focus issue and reference that in the comment in the patch where I've left xxxxxx.

Comment 6

[David :Bienvenu]

I was able to reproduce this; I'll try the fix out.

need to fix the xxxxxx bug, and spelling of focused.

+	// XXX If the message pane is selected but the tab focussed, this ends
+        // closing the message tab. See bug xxxxxx.

Comment 7

[David :Bienvenu]

Comment on attachment 387188 [details] [diff] [review]
Proposed improvement

r/sr=me, modulo prev comments.

Comment 8

[Mark Banner (:standard8)]

Checked in with comments addressed: http://hg.mozilla.org/comm-central/rev/10d929a67931