Closed Bug 955237 Opened 10 years ago Closed 10 years ago

JS error at shutdown when exiting with the Command+Q shortcut in a conversation window

Categories

(Instantbird Graveyard :: Conversation, defect)

x86
Other
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: florian)

Details

Attachments

(1 file)

*** Original post on bio 1804 at 2012-11-17 23:51:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Attached patch PatchSplinter Review
*** Original post on bio 1804 as attmnt 2095 at 2012-11-17 23:51:00 UTC ***

The reported error is:
JavaScript Error: "this.target is undefined" {file: "file:///Users/florian/buildhg/obj-instantbird-dbg/mozilla/dist/InstantbirdDebug.app/Contents/MacOS/components/imConversations.js" line: 272}

The error occurs when attempting to send typing notifications from a setTimeout callback that was started because we pressed a key in the conversation binding.

The setTimeout uses a .bind() that keeps the conversation.xml binding alive so the destructor isn't called, so _forgetConv isn't called, so this._conv isn't null, even though the conversation service is already gone.

I'm tired of these non-actionable "this.target is undefined" errors that generally mean that something in a conversation binding is still trying to do something to an imConversation that's already been destroyed. (See bug 955166 (bio 1735) for another instance of that unhelpful error message.)

The attached patch ensures that the conversation.xml binding drops the reference to this._conv if it's been destroyed.
It may not fix all the bugs causing the binding to attempt to access this._conv after it's been destroyed, but at least the JS errors will now reference the code wrongly accessing that, instead of some non-sense in the conversations service.

And as the typing notification code actually has a null check on this._conv, this patch also really fixes the error that caused me to look into this.
Attachment #8353856 - Flags: review?(aleth)
Comment on attachment 8353856 [details] [diff] [review]
Patch

*** Original change on bio 1804 attmnt 2095 at 2012-11-18 12:44:22 UTC ***

There are a few open bugs with "this.target undefined" too, with luck this patch will help with those as well :)

This if clause
http://lxr.instantbird.org/instantbird/source/instantbird/content/conversation.xml#94
should be moved outside the if (this.conv) check.
Attachment #8353856 - Flags: review?(aleth) → review-
*** Original post on bio 1804 at 2012-11-18 19:01:19 UTC ***

Per http://log.bezut.info/instantbird/121118/#m131

http://hg.instantbird.org/instantbird/rev/98fd6cfae591
Assignee: nobody → florian
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4
You need to log in before you can comment on or make changes to this bug.