Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Add a findbar and support the zoom feature in the chat tab

RESOLVED FIXED in Thunderbird 15.0

Status

Thunderbird
Instant Messaging
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: florian, Assigned: florian)

Tracking

Trunk
Thunderbird 15.0

Thunderbird Tracking Flags

(thunderbird14 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
Created attachment 619094 [details] [diff] [review]
Patch

I was mostly interested in adding the findbar as we will want to use it for a follow up to bug 743235, but while looking at how the find commands work in other Thunderbird tabs, I noticed I could get the zoom working almost for free, and as we will want it too anyway, I put both in the same bug and patch.
Attachment #619094 - Flags: review?(bwinton)
Comment on attachment 619094 [details] [diff] [review]
Patch

I just noticed something which you might want to fix in this patch. Ctrl-K doesn't focus the global search bar, even though the empty text says it does.

>+++ b/mail/components/im/content/chat-messenger-overlay.js
>@@ -124,20 +124,93 @@ var chatTabType = {
>+  supportsCommand: function supportsCommand(aCommand, aTab) {

nit: I think we usually name these "ct_supportsCommand", just so that we can more easily see where they're coming from.

>+  doCommand: function isCommandEnabled(aCommand, aTab) {

And this function really shouldn't be named "isCommandEnabled".  ;)

>+  getBrowser: function getBrowser(aTab) {
>+    let panel = document.getElementById("conversationsDeck").selectedPanel;
>+    if (panel == document.getElementById("logDisplay")) {
>+      if (document.getElementById("logDisplayDeck").selectedPanel ==
>+          document.getElementById("logDisplayBrowserBox"))
>+        return document.getElementById("conv-log-browser");
>+    }
>+    else if (panel && panel.localName == "imconversation")
>+      return panel.browser;

As mentioned in a previous review, there should be {}s around this else clause (and the one a little further down).

Other than that I'm pretty happy.  r=me, and ui-r=me.

Later,
Blake.
Attachment #619094 - Flags: ui-review+
Attachment #619094 - Flags: review?(bwinton)
Attachment #619094 - Flags: review+
(Assignee)

Comment 2

5 years ago
(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #1)
> Comment on attachment 619094 [details] [diff] [review]
> Patch
> 
> I just noticed something which you might want to fix in this patch. Ctrl-K
> doesn't focus the global search bar, even though the empty text says it does.

I have a patch ready for check-in addressing this specific issue in bug 749552.
(Assignee)

Comment 3

5 years ago
Created attachment 619928 [details] [diff] [review]
Patch for checkin

Addresses the nits from comment 1.
(Assignee)

Comment 4

5 years ago
http://hg.mozilla.org/comm-central/rev/60c7b4af09cb
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 15.0
(Assignee)

Updated

5 years ago
Attachment #619928 - Flags: approval-comm-aurora?

Updated

5 years ago
Attachment #619928 - Flags: approval-comm-aurora? → approval-comm-aurora+
Checked into aurora:

https://hg.mozilla.org/releases/comm-aurora/rev/cd0ce382db04
status-thunderbird14: --- → fixed
You need to log in before you can comment on or make changes to this bug.