1569399 - entity openConversationButton.tooltip used twice, causes wrong tooltip for conversation button in main toolbar to be shown

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Daily 70.0a1 20190726083520 and Thunderbird 68.0b5 on Windows 8.1

The entity openConversationButton.tooltip is used twice (for chat conversations and threaded mail conversations) which causes the wrong tooltip to be shown for the conversation button in the main toolbar.

Steps to reproduce:

1. Customize main toolbar.
2. Add 'Conversation' button.
3. End customization.
4. Move mouse over button.

Actual result:
Tooltip "Start a conversation" (mail/locales/en-US/chrome/messenger/chat.dtd)

Expected result:
Tooltip "Show conversation of selected message" (mail/locales/en-US/chrome/messenger/messenger.dtd)

Occurrences in code: https://dxr.mozilla.org/comm-release/search?q=openConversationButton&redirect=false

Comment 1

[Jorg K (CEST = GMT+2)]

Since when do we use this twice? Is this working in TB 68? Anyway, too late to fix it there.

Khushil has done quite some work in chat, or else Richard may have some spare time.

Comment 2

[Khushil Mistry [:khushil324]]

Should I create the patch for TB68 or Daily or both?

Comment 3

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

If you use a new entity, only for Daily. If you correct the loading order/scopes (= no new entity necessary), then for both, please. Thank you.

Comment 4

[Khushil Mistry [:khushil324]]

Attachment: Bug-1569399_openConversationButton-tooltip-edited.patch

Comment 5

[Jorg K (CEST = GMT+2)]

Comment on attachment 9081224 [details] [diff] [review]
Bug-1569399_openConversationButton-tooltip-edited.patch

Looks good to me. Sadly not backportable since it's a string change, that is, you're creating a new string openMsgConversationButton.

Comment 6

[Jorg K (CEST = GMT+2)]

Coming back to Aryx' suggestion:
https://dxr.mozilla.org/comm-release/search?q=openConversationButton&redirect=false

So openConversationButton is used in mail/locales/en-US/chrome/messenger/chat.dtd and mail/locales/en-US/chrome/messenger/messenger.dtd.

Khushil, can you try a version for TB 69 and TB 68 where you move chat.dtd after messenger.dtd here:
https://searchfox.org/comm-central/rev/84706b631aafd18e54074588efb71ddf91293ee6/mail/base/content/messenger.xul#30-34

Of course we can land the proper fix on trunk, but we should consider fixing the branches as well. Or do the reshuffling on all branches and additionally fix the string on trunk.

I'll wait for your reply before landing this.

Comment 7

[Jorg K (CEST = GMT+2)]

Actually, I have a patch, I'll let you review it if it works.

Comment 8

[Jorg K (CEST = GMT+2)]

Attachment: 1569399-beta-esr68.patch

This fixes the Conversation button after being placed onto a toolbar. I can't see where the "start a conversation" button is used. Anyone knows?

Comment 9

[Khushil Mistry [:khushil324]]

Comment on attachment 9081356 [details] [diff] [review]
1569399-beta-esr68.patch

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

It works for me. r=khushil.

Comment 10

[Jorg K (CEST = GMT+2)]

Comment on attachment 9081356 [details] [diff] [review]
1569399-beta-esr68.patch

This needs uplift to fix the incorrect tooltip.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/eaa12f8b4d9e
Move chat.dtd after messenger.dtd in messenger.xul to work around duplicate string issue. r=khushil
https://hg.mozilla.org/comm-central/rev/050d0ef127f2
openConversationButton.tooltip changed to openMsgConversationButton.tooltip for messenger.dtd to fix duplication. r=jorgk

Comment 12

[Jorg K (CEST = GMT+2)]

TB 69 beta 2:
https://hg.mozilla.org/releases/comm-beta/rev/c11139a488bed744ca3b24ec0e7cf16f4c1ddcfe

Comment 13

[Jorg K (CEST = GMT+2)]

TB 68.0 ESR:
https://hg.mozilla.org/releases/comm-esr68/rev/c4e93aa915a3bf1b6930dbc5ae828f36f2ba0922