1559793 - OTR: JS exception in OTRUI.jsm:497, this.globalDoc is null

Status: RESOLVED FIXED

(Chat Core :: Security: OTR, defect, P1)

Description

[Kai Engert (:KaiE:)]

When receiving an OTR message, but the conversation window hasn't been loaded yet, we get a call to OTRUI.notifyUnverified(), but this.globalDoc hasn't been set yet.

Two consequences:

• exception in JS console
• yellow notification box not shown

Comment 1

[Alessandro Castellani [:aleca]]

Attachment: 1559793-otrui.patch

This patch fixes the issue by forcing the opening of the Chat Tab if it hasn't been opened before.
I think we're forced to open the Chat Tab in order to initialize the OTR, which it can't happen without the loaded UI. Am I wrong?

Another UX related issue I noticed is that if the user has the Chat Tab opened, but the focus is on another Tab, the request of verification doesn't produce any visible notification, like a count bubble or the highlight of the conversation.
The Notification Box with the request for verification gets properly generated tho, so it should be only a matter of hooking up the OTR to use the same chat bubble notification happening in a regular conversation.
Will do that in a follow up bug.

P.S.: I had to wrap all the conditions around brackets as eslint was screaming at me.

Comment 2

[Kai Engert (:KaiE:)]

(In reply to Alessandro Castellani (:aleca) from comment #1)

This patch fixes the issue by forcing the opening of the Chat Tab if it hasn't been opened before.

Thanks, the patch fixes the issue for me.

I suggest a small rewrite to avoid the duplicate call to OTRUI.notifyUnverified:

        if (!this.globalDoc) {
          let win = Services.wm.getMostRecentWindow("mail:3pane");
          if (!win) {
            return;
          }
          win.focus();
          win.showChatTab();
          this.globalDoc = win.document;
        }
        OTRUI.notifyUnverified(aObject, aMsg);
        break;

I think we're forced to open the Chat Tab in order to initialize the OTR, which it can't happen without the loaded UI. Am I wrong?

I'm fine with opening the chat tab in this scenario.

To answer your question, receiving of OTR messages works just fine, even with the chat tab closed. As an example, if your buddy is already verified, your code path isn't triggered. If the chat tab isn't open yet, the OTR messages are received fine.

This means, with your change, if the chat tab is closed:

• receiving an OTR message from an unverified contact will open the chat tab
• receiving an OTR message from a verified contact will NOT open the chat tab. Rather, the status bar icon for chats will show a red notification icon.

It's a small inconsistency, but I think fixing this bug is more important. I recommend to take your patch. If you'd like to improve the inconsistency, we can do that in a follow-up bug.

Another UX related issue I noticed is that if the user has the Chat Tab opened, but the focus is on another Tab, the request of verification doesn't produce any visible notification, like a count bubble or the highlight of the conversation.
The Notification Box with the request for verification gets properly generated tho, so it should be only a matter of hooking up the OTR to use the same chat bubble notification happening in a regular conversation.
Will do that in a follow up bug.

Thanks!

Comment 3

[Kai Engert (:KaiE:)]

Comment on attachment 9076255 [details] [diff] [review]
1559793-otrui.patch

r=kaie, but removing the flag, because of the suggested change

Comment 4

[Kai Engert (:KaiE:)]

Attachment: 1559793-otrui-v2.patch

Alex, please set r+ if you agree with my small change.

Then we'll need checkin-needed and approval-esr68

Comment 5

[Jorg K (CEST = GMT+2)]

Comment on attachment 9076862 [details] [diff] [review]
1559793-otrui-v2.patch

no uplifts directly to comm-esr68 for the moment, I'll do a bulk move from the 68 beta. What a pain we don't have a release flag to distinguish the beta 68 from beta 69 :-(

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/42e87103b398
Fix OTRUI.jsm exception this.globalDoc is null. r=kaie

Comment 7

[Jorg K (CEST = GMT+2)]

TB 69 beta (TB 68 beta coming):
https://hg.mozilla.org/releases/comm-beta/rev/98ee2d4ceaca634121e90539e09d4a4b12274887

Comment 8

[Jorg K (CEST = GMT+2)]

TB 68 beta 5:
https://hg.mozilla.org/releases/comm-beta/rev/db02405147d0bf65623884eb4f9add11468b6d23