Debug log tab searches in wrong direction

RESOLVED FIXED in 1.6

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: aleth, Assigned: nhnt11)

Tracking

trunk
x86
macOS

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
STR: Repeat search in a debug log tab using the keyboard will search upwards instead of downwards, which is confusing.

Probably due to having copied the findbar from a convbrowser.
(Assignee)

Comment 1

4 years ago
Posted patch Patch (obsolete) — Splinter Review
This fixes the issue.
Let me know if the comment is good enough.
Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
Attachment #8557649 - Flags: review?(aleth)
(Assignee)

Comment 2

4 years ago
A bit of detail: the reversed searching happens because the find commands in instantbird.xul assume that all panels are conversations. The relevant keyboard shortcuts are Cmd(+Shift)+G.
(Reporter)

Comment 3

4 years ago
Comment on attachment 8557649 [details] [diff] [review]
Patch

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

Thanks for taking care of this!

::: im/content/instantbird.xul
@@ +77,5 @@
> +                          return;
> +                        }
> +                        var panel = getTabBrowser().selectedPanel;
> +                        if (!panel) return;
> +                        if (panel.findbar) panel.findbar.onFindAgainCommand(false);"/>

I'm not sure, but would it be clearer to do something like

var findbar, isReversedDirection = false;
var conv = getTabBrowser.selectedConversation;
if (conv) {
  isReversedDirection = true;
  findbar = conv.findbar;
}
else {
  var panel = getTabbrowser.selectedPanel;
  if (!panel)
    return;
  findbar = panel.findbar;
}
findbar.onFindAgainCommand(isReversedDirection);
Attachment #8557649 - Flags: review?(aleth) → review+
(Assignee)

Comment 4

4 years ago
Posted patch Patch v2Splinter Review
Moves the code to a function that accepts one parameter to avoid duplication.
Attachment #8557649 - Attachment is obsolete: true
Attachment #8564456 - Flags: review?(aleth)
(Reporter)

Comment 5

4 years ago
Comment on attachment 8564456 [details] [diff] [review]
Patch v2

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

Thanks!
Attachment #8564456 - Flags: review?(aleth) → review+
(Reporter)

Updated

4 years ago
Keywords: checkin-needed
(Reporter)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 1.6
You need to log in before you can comment on or make changes to this bug.