Closed Bug 1404706 Opened 4 years ago Closed 4 years ago

Chat shows no conversations

Categories

(Thunderbird :: Instant Messaging, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 58.0

People

(Reporter: Paenglab, Assigned: jorgk-bmo)

Details

Attachments

(1 file, 2 obsolete files)

I first thought the culprit was the same as bug 1404003 because it appeared at the same time. But bug 1404003 is fixed and this one one still happens.

Errors I get when connecting to a chat server:

ReferenceError: reference to undefined property "getScriptableHelper"[Learn More]  imXPCOMUtils.jsm:89:5
ReferenceError: reference to undefined property "headersSize"[Learn More]  network-monitor.js:596:5
ReferenceError: reference to undefined property "fill"[Learn More]  ToLocaleFormat.jsm:163:11
Error in parsing value for ‘unicode-bidi’.  Declaration dropped.  conv.css:31:16
ReferenceError: reference to undefined property "nsIDOMHTMLAnchorElement"[Learn More]  imSmileys.jsm:161:1
TypeError: invalid 'instanceof' operand Components.interfaces.nsIDOMHTMLAnchorElement  imSmileys.jsm:161:1

Actually I'm building on M-C at cb604e7830ec and C-C at 3f2554599137 to see if this is already broken there.
nsIDOMHTMLAnchorElement was removed in bug 1403516 / bug 1389650. That's pretty bad since the IDL interface was removed altogether, so
  if (testNode instanceof Components.interfaces.nsIDOMHTMLAnchorElement &&
will not work anymore.

ToLocaleFormat.jsm:163 is
  let {fill = "", width = 0} = padding[specifier] || {};
so some JS problem for Aceman.

network-monitor.js:596 is M-C:
  response.transferredSize = this.transferredSize + this.httpActivity.headersSize;

imXPCOMUtils.jsm:89
  this.imAccount.logDebugMessage(scriptError, aLevel);
doesn't have getScriptableHelper, hmm.

Aceman, any spare cycles for this bustage?
Flags: needinfo?(acelists)
Attached patch 1404706-chat-bustage.patch - WIP (obsolete) — Splinter Review
Fixed for the C-C errors, let's see how far we get.
Flags: needinfo?(acelists)
OK, with this patch I see:

*************************
A coding exception was thrown and uncaught in a Task.

Full message: TypeError: Services.logs is undefined
Full stack: _beginIndexingJob/<@resource:///modules/index_im.js:390:1
TaskImpl_run@resource://gre/modules/Task.jsm:331:42
TaskImpl@resource://gre/modules/Task.jsm:280:3
asyncFunction@resource://gre/modules/Task.jsm:252:14
Task_spawn@resource://gre/modules/Task.jsm:166:12
_beginIndexingJob@resource:///modules/index_im.js:386:5
notify@resource:///modules/imXPCOMUtils.jsm:148:33

*************************

Let's fix that first.
I have a patch that shows the conversations again, but there is another error:

*************************
A coding exception was thrown and uncaught in a Task.

Full message: TypeError: logFiles is null
Full stack: _beginIndexingJob/<@resource:///modules/index_im.js:392:11
TaskImpl_run@resource://gre/modules/Task.jsm:331:42
promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7
TaskImpl_run@resource://gre/modules/Task.jsm:339:15
TaskImpl@resource://gre/modules/Task.jsm:280:3
asyncFunction@resource://gre/modules/Task.jsm:252:14
Task_spawn@resource://gre/modules/Task.jsm:166:12
_beginIndexingJob@resource:///modules/index_im.js:387:5
notify@resource:///modules/imXPCOMUtils.jsm:148:33

*************************

That's from
  if (!logFiles.length) {

so let's fix this while I'm here.
Hmm, I still get:

[5912, Main Thread] WARNING: Failed to open external DTD: publicId "-//Apple//DTD PLIST 1.0//EN" systemId "http://www.apple.com/DTDs/PropertyList-1.0.dtd" base "moz-nullprincipal:{3bac946d-da47-422f-a5c0-2c6732aa5f0a}" URL "resource://gre/res/dtd/PropertyList-1.0.dtd": file c:/mozilla-source/comm-central/mozilla/parser/htmlparser/nsExpatDriver.cpp, line 700
Chrome file doesn't exist: C:\mozilla-source\comm-central\obj-x86_64-pc-mingw32\dist\bin\chrome\messenger\skin\classic\messenger\messages\Header.html
Chrome file doesn't exist: C:\mozilla-source\comm-central\obj-x86_64-pc-mingw32\dist\bin\chrome\messenger\skin\classic\messenger\messages\Incoming\NextContext.html
Chrome file doesn't exist: C:\mozilla-source\comm-central\obj-x86_64-pc-mingw32\dist\bin\chrome\messenger\skin\classic\messenger\messages\Outgoing\Content.html
Chrome file doesn't exist: C:\mozilla-source\comm-central\obj-x86_64-pc-mingw32\dist\bin\chrome\messenger\skin\classic\messenger\messages\Outgoing\Context.html
Chrome file doesn't exist: C:\mozilla-source\comm-central\obj-x86_64-pc-mingw32\dist\bin\chrome\messenger\skin\classic\messenger\messages\Outgoing\NextContent.html
Chrome file doesn't exist: C:\mozilla-source\comm-central\obj-x86_64-pc-mingw32\dist\bin\chrome\messenger\skin\classic\messenger\messages\Outgoing\NextContext.html

Weird.
Attached patch 1404706-chat-bustage.patch (v2) (obsolete) — Splinter Review
Richard, please try this.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8914074 - Flags: review?(acelists)
Attachment #8914074 - Flags: feedback?(richard.marti)
OK, using 'in' by popular demand.
Attachment #8914070 - Attachment is obsolete: true
Attachment #8914074 - Attachment is obsolete: true
Attachment #8914074 - Flags: review?(acelists)
Attachment #8914074 - Flags: feedback?(richard.marti)
Attachment #8914075 - Flags: review?(acelists)
Attachment #8914075 - Flags: feedback?(richard.marti)
Comment on attachment 8914075 [details] [diff] [review]
1404706-chat-bustage.patch (v2b)

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

Thanks, works for me to join IRC.

::: chat/modules/imSmileys.jsm
@@ +157,5 @@
>     * Check the class name to skip any autolinked nodes from mozTXTToHTMLConv.
>     */
>    let testNode = aNode;
>    while ((testNode = testNode.parentNode)) {
> +    if (testNode.localName == "a" &&

The docs say localName is deprecated and should not be used.
I'm investigating whether this should be 'instance of HTMLAnchorElement' or '.nodeName.toLowerCase() == "a"'
Attachment #8914075 - Flags: review?(acelists) → review+
Comment on attachment 8914075 [details] [diff] [review]
1404706-chat-bustage.patch (v2b)

Works. :)
Attachment #8914075 - Flags: feedback?(richard.marti) → feedback+
(In reply to :aceman from comment #8)
> The docs say localName is deprecated and should not be used.
We use it a lot.

> I'm investigating whether this should be 'instance of HTMLAnchorElement' or
That's not an IDL defined interface, so no.

> '.nodeName.toLowerCase() == "a"'
OK. tagName on the other hand is only for elements and not every node is an element.
(In reply to Jorg K (GMT+2) from comment #10)
> (In reply to :aceman from comment #8)
> > The docs say localName is deprecated and should not be used.
> We use it a lot.

Yes, but we may need to replace it at some point.
https://developer.mozilla.org/en-US/docs/Web/API/Node/localName
 
> > I'm investigating whether this should be 'instance of HTMLAnchorElement' or
> That's not an IDL defined interface, so no.

If you look in dxr this one is used many times and even in c-c already. So it must be something.
 
> > '.nodeName.toLowerCase() == "a"'
> OK. tagName on the other hand is only for elements and not every node is an
> element.

Yes.
(In reply to :aceman from comment #11)
> > > I'm investigating whether this should be 'instance of HTMLAnchorElement' or
> > That's not an IDL defined interface, so no.
> If you look in dxr this one is used many times and even in c-c already. So
> it must be something.
Hmm, I was wrong. So why don't we just use
-    if (testNode instanceof Components.interfaces.nsIDOMHTMLAnchorElement &&
+    if (testNode instanceof HTMLAnchorElement &&
?
(In reply to Jorg K (GMT+2) from comment #12)
> Hmm, I was wrong. So why don't we just use
> -    if (testNode instanceof Components.interfaces.nsIDOMHTMLAnchorElement &&
> +    if (testNode instanceof HTMLAnchorElement &&
> ?

Because I tried it and it was undefined in that particular file. So I wonder how it works in the other c-c files, where does it come from. That's what I asked in bug 1403516 comment 31.
Hmm ...
JavaScript error: resource:///modules/imSmileys.jsm, line 161: ReferenceError: HTMLAnchorElement is not defined

OK, back to nodeName :-(
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/9dafe6d1a064
Port bug 1389650 (removal of nsIDOMHTMLAnchorElement) to chat and fix misc. JS errors. r=aceman
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 58.0
In a module, you could do this:
(node instanceof node.ownerGlobal.HTMLAnchorElement)
You need to log in before you can comment on or make changes to this bug.