Closed Bug 1305008 Opened 3 years ago Closed 3 years ago

set _viewFlags does not check whether this.dbView == null

Categories

(MailNews Core :: Backend, defect, critical)

defect
Not set
critical

Tracking

(thunderbird50 fixed, thunderbird_esr4550+ fixed, thunderbird51 fixed, thunderbird52 fixed)

RESOLVED FIXED
Thunderbird 52.0
Tracking Status
thunderbird50 --- fixed
thunderbird_esr45 50+ fixed
thunderbird51 --- fixed
thunderbird52 --- fixed

People

(Reporter: ps.stoecker, Assigned: alta88)

References

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20160920090324

Steps to reproduce:

The bug occurred in the AddOn thunderbird-conversations ( see https://github.com/protz/thunderbird-conversations/issues/1114)

When the user runs the setup assistant the first time, the setup is never completed.


Actual results:

StackTrace:

2016-09-23 01:28:33 Conversations.AssistantUI ERROR Error in customization ctionSetupView
2016-09-23 01:28:33 Conversations.AssistantUI ERROR TypeError: this.dbView is null
2016-09-23 01:28:33 Conversations DEBUG
DBViewWrapper.prototype._viewFlags@resource://gre/modules/dbViewWrapper.js:1308:1
DBViewWrapper.prototype.showThreaded@resource://gre/modules/dbViewWrapper.js:1724:7
MsgSortThreaded@chrome://messenger/content/threadPane.js:267:3
Customizations.actionSetupView.install/moveOn@resource://conversations/modules
/assistant.js:202:9
Customizations.actionSetupView.install/waitForIt@resource://conversations/modules/assistant.js:221:11
Customizations.actionSetupView.install@resource://conversations/modules/assistant.js:225:7
install@chrome://conversations/content/assistant/assistant.xhtml:32:31
onFinish@chrome://conversations/content/assistant/assistant.xhtml:83:11
onclick@chrome://conversations/content/assistant/assistant.xhtml:1:1

I think the bug was introduced in https://bugzilla.mozilla.org/show_bug.cgi?id=1192838

In mail/base/modules/dbViewWrapper.js:1290 -- 1314 the null-Check for this.dbView is done (line 1311) after the first access (line 1308). According to the comment above the function the case for this.dbView==null should be handled, but it isn't.


Expected results:

null-Check should be done before first access and no exception should be thrown
See Also: → 1192838
See Also: → 978559
Thanks for looking out for this. And good research!


(In reply to ps.stoecker from comment #0)
> User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101
> Firefox/49.0
> Build ID: 20160920090324
> 
> Steps to reproduce:
> 
> The bug occurred in the AddOn thunderbird-conversations ( see
> https://github.com/protz/thunderbird-conversations/issues/1114)
> 
> When the user runs the setup assistant the first time, the setup is never
> completed.
> 
> 
> Actual results:
> 
> StackTrace:
> 
> 2016-09-23 01:28:33 Conversations.AssistantUI ERROR Error in customization
> ctionSetupView
> 2016-09-23 01:28:33 Conversations.AssistantUI ERROR TypeError: this.dbView
> is null
> 2016-09-23 01:28:33 Conversations DEBUG
> DBViewWrapper.prototype._viewFlags@resource://gre/modules/dbViewWrapper.js:
> 1308:1
> DBViewWrapper.prototype.showThreaded@resource://gre/modules/dbViewWrapper.js:
> 1724:7
> MsgSortThreaded@chrome://messenger/content/threadPane.js:267:3
> Customizations.actionSetupView.install/moveOn@resource://conversations/
> modules
> /assistant.js:202:9
> Customizations.actionSetupView.install/waitForIt@resource://conversations/
> modules/assistant.js:221:11
> Customizations.actionSetupView.install@resource://conversations/modules/
> assistant.js:225:7
> install@chrome://conversations/content/assistant/assistant.xhtml:32:31
> onFinish@chrome://conversations/content/assistant/assistant.xhtml:83:11
> onclick@chrome://conversations/content/assistant/assistant.xhtml:1:1
> 
> I think the bug was introduced in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1192838

Perhaps you could confirm this by testing https://archive.mozilla.org/pub/thunderbird/nightly/2015/11/2015-11-12-03-04-27-comm-central/ which does NOT have the patch, and https://archive.mozilla.org/pub/thunderbird/nightly/2015/11/2015-11-13-03-02-34-comm-central/ which DOES have the patch.

> 
> In mail/base/modules/dbViewWrapper.js:1290 -- 1314 the null-Check for
> this.dbView is done (line 1311) after the first access (line 1308).
> According to the comment above the function the case for this.dbView==null
> should be handled, but it isn't.
> 
> 
> Expected results:
> 
> null-Check should be done before first access and no exception should be
> thrown

There is I think at least one other bug report about this. Can you get a crash ID?
https://archive.mozilla.org/pub/thunderbird/nightly/2015/11/2015-11-13-03-02-34-comm-central/

And, FWIW the whole dbView area is very crashy for us. Just look at https://mzl.la/2cIgZCh :( :(
Severity: normal → critical
Status: UNCONFIRMED → NEW
Component: Untriaged → Backend
Ever confirmed: true
Flags: needinfo?(ps.stoecker)
Keywords: regression
Product: Thunderbird → MailNews Core
Version: 45 Branch → 45
Attached patch viewflags.patch (obsolete) — Splinter Review
(In reply to ps.stoecker from comment #0)
> 
> Expected results:
> 
> null-Check should be done before first access and no exception should be
> thrown

indeed it should.
Assignee: nobody → alta88
Attachment #8794175 - Flags: review?(mkmelin+mozilla)
Attached patch viewflags.patch (obsolete) — Splinter Review
Attachment #8794175 - Attachment is obsolete: true
Attachment #8794175 - Flags: review?(mkmelin+mozilla)
Attachment #8794176 - Flags: review?(mkmelin+mozilla)
Thanks for the quick response and the patch :-)
Maybe "crash" was the wrong wording - as the exception was caught in the AddOn's code, the program continued. Hence, there is no crash ID.

https://archive.mozilla.org/pub/thunderbird/nightly/2015/11/2015-11-11-03-02-40-comm-central/ does not seem to contain any linux builds - so do I have to compile it myself?
yes, crash is not what happens, but things won't be right due to the exception. unless you have a comm-central checked out and know how to apply patches/build it, perhaps waiting for this to land and getting a nightly afterwards might be easier.
Comment on attachment 8794176 [details] [diff] [review]
viewflags.patch

Review of attachment 8794176 [details] [diff] [review]:
-----------------------------------------------------------------

r=mkmelin but please update the commit message as it's not a crash
Attachment #8794176 - Flags: review?(mkmelin+mozilla) → review+
Attached patch viewflags.patchSplinter Review
Attachment #8794176 - Attachment is obsolete: true
Attachment #8794350 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/b1c8adff7443

Uplifts?
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(ps.stoecker)
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 52.0
Summary: set _viewFlags crashes if this.dbView == null → set _viewFlags does not check whether this.dbView == null
Comment on attachment 8794350 [details] [diff] [review]
viewflags.patch


I haven't observed this error in stock Tb, but the patch is no risk, and the extension is rather popular.
Attachment #8794350 - Flags: approval-comm-esr45?
Comment on attachment 8794350 [details] [diff] [review]
viewflags.patch

In this case we need to uplift to Aurora and Beta first.
Attachment #8794350 - Flags: approval-comm-beta+
Attachment #8794350 - Flags: approval-comm-aurora+
I can confirm that the bug does not occur in the latest nightly.
Is there a chance that the fix will be applied in tb 45 ?
(In reply to ps.stoecker from comment #12)
> I can confirm that the bug does not occur in the latest nightly.
> Is there a chance that the fix will be applied in tb 45 ?

It is likely this patch will be included in the next release of Thunderbird 45. All the necessary preliminaries seem to be present here.
Comment on attachment 8794350 [details] [diff] [review]
viewflags.patch

https://hg.mozilla.org/releases/comm-esr45/rev/59e9a33f6c0c
Attachment #8794350 - Flags: approval-comm-esr45? → approval-comm-esr45+
You need to log in before you can comment on or make changes to this bug.