1559097 - Keyboard shortcuts Ctrl+Up and Ctrl+Down to switch between conversations are broken in TB68.0b1

Status: RESOLVED FIXED

(Thunderbird :: Instant Messaging, defect)

Description

[Onno Ekker [:nONoNonO UTC+1]]

In Thunderbird 68.0b1 the keyboard shortcuts Ctrl+Up and Ctrl+Down to switch between convversations are broken, probably because not all of the de-XBL-changes have made it into this beta yet. The keys do work in Thunderbird 60.7.0 ESR and in current Daily.

Steps to reproduce:

1. Start Chat with multiple conversations, e.g. IRC with multiple channels and the server notifications;
2. Press Ctrl+Up or Ctrl+Down to switch to the previous/next conversation;
3. Notice that nothing happens

Comment 1

[Jorg K (CEST = GMT+2)]

68 beta is a bad mix of XBL and de-XBL code. In order to fix bug 1555136 on beta, which is fixed on trunk by bug 1538547, I backed out bug 1538548, bug 1538550, bug 1538549 from beta. I tried landing bug 1538547 on beta, but unlike on trunk, it didn't fix the issue.

See bug 1555136 comment #23 for a summary.

I'm not sure why it's not working on beta, since as far as chat is concerned, it's fully back to XBL code, unless I'm missing something.

Comment 2

[Jorg K (CEST = GMT+2)]

Khushil, could you please take a look at this. Working on trunk, not working on beta. So we need to see where in the code Ctrl+U/D is handled and check why it's not working on beta. Browse beta code here: https://dxr.mozilla.org/comm-beta/source/

Comment 3

[Khushil Mistry [:khushil324]]

Yeah, sure. I will check it and submit the patch.

Comment 4

[Jorg K (CEST = GMT+2)]

Any progress? We're building TB 68 beta 4 this week.

Comment 5

[Khushil Mistry [:khushil324]]

Yeah, I am working on it. Hopefully will submit by today itself.

Comment 6

[Khushil Mistry [:khushil324]]

Attachment: Bug-1559097_fixing_shortcuts-chat.patch

This is because of Richlistbox in the mozilla-esr68 is a Custom element and has this method : https://hg.mozilla.org/releases/mozilla-esr68/file/97e53c49e30125bd4cdce0c05e858fb3c021690e/toolkit/content/widgets/richlistbox.js#l263
So we need to override method get itemChildren() because name of our richlistitems are imconv, imgroup and imcontact. So I have created a custom element to override get itemChildren().

Comment 7

[Khushil Mistry [:khushil324]]

Attachment: Bug-1559097_fixing_shortcuts-chat.patch

Comment 8

[Jorg K (CEST = GMT+2)]

Thanks, I don't build beta locally, so I've started a try build:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=394573f3e912d95baa7f40a080870479b218a7f5

Looking at the patch, contact-richlistbox.js has four-spaces indentation. We use two, right?

Comment 9

[Khushil Mistry [:khushil324]]

(In reply to Jorg K (GMT+2) from comment #8)

Looking at the patch, contact-richlistbox.js has four-spaces indentation. We use two, right?

Yess, I will submit the updated file with the final review.

Comment 10

[Jorg K (CEST = GMT+2)]

Well, Magnus is on PTO, so I will take this beta-only patch if it works. So either you or me need to fix the indentation pretty much now ;-)

Comment 11

[Khushil Mistry [:khushil324]]

Yes, submitting right now.

Comment 12

[Khushil Mistry [:khushil324]]

Attachment: Bug-1559097_fixing_shortcuts-chat.patch

Comment 13

[Jorg K (CEST = GMT+2)]

Comment on attachment 9075793 [details] [diff] [review]
Bug-1559097_fixing_shortcuts-chat.patch

Thanks, I'll stick the r+ onto it when I see it working. How did you test it? Did you build beta locally?

Comment 14

[Khushil Mistry [:khushil324]]

(In reply to Jorg K (GMT+2) from comment #13)

Thanks, I'll stick the r+ onto it when I see it working. How did you test
it? Did you build beta locally?

Yes, I have built the beta on linux machine locally.

Comment 15

[Jorg K (CEST = GMT+2)]

Comment on attachment 9075793 [details] [diff] [review]
Bug-1559097_fixing_shortcuts-chat.patch

Thanks, works for me. I'll leave the r? for Magnus, he'll take a look when he returns. Anyway, this is beta-only, so we're not committing anything to the codebase that will live forever.

Comment 16

[Jorg K (CEST = GMT+2)]

TB 68 beta 4:
https://hg.mozilla.org/releases/comm-beta/rev/0a52f45d157aecdc035214a11ea306816f915b1b