328795 - Convert Thunderbird to use typeahead find

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Scott MacGregor]

Get rid of the find dialog and use tookit's type ahead find widget for searching messages.

Comment 1

[Scott MacGregor]

this depends on 325416.

Comment 2

[Scott MacGregor]

Attachment: the fix

Use the find toolbar instead of the old find dialog.

Comment 3

[Scott MacGregor]

Note to self: when this lands, we should remove "Find in Message" from the quick search bar and all of the related DOM highlighting magic there. That is all completely redundant when you have the find in message bar which does the same thing. 

Comment 4

[Scott MacGregor]

Attachment: follow up fix to remove find in message and highlighting code from QS

As promised here's the follow up patch.

1) Remove find in message from the quick search bar. This is duplicating functionality from the find bar.

2) Remove the highlighting code from searchBar.js. We don't need this anymore. Highlighting is done from the find bar.

3) in search.xml, when we are constructing the search widget, account for the case that the desired quick search mode we remembered in localstore.rdf may not be there anymore (as is the case with find in message :)), so just use the first search mode in the list in this case.

4) While I was in search.xml, I fixed an old bug where the quick search menupopup doesn't properly check the right search mode when you first start up. It always showed Subject or Sender.

Comment 5

[David :Bienvenu]

Comment on attachment 213814 [details] [diff] [review]
follow up fix to remove find in message and highlighting code from QS

nice to see code removed :-)

+  // If the find bar is visible and we just loaded a new message, re-run 
+  // the find command. This means the new message will get highlighted and
+  // we'll scroll to the first word in the message that matches the find text.
+  if (gFindBar.isFindBarVisible())
+    gFindBar.find();


I guess we'll see how this feels. This will happen in any view, right?

Comment 6

[Scott MacGregor]

both patches are in both the trunk and 1.8.1 branch now. 

Comment 7

[Scott MacGregor]

I also added fastfind.xpt to the packages-static file for windows builds. 

Comment 8

[Mike Cowperthwaite]

(In reply to comment #4)
> 4) While I was in search.xml, I fixed an old bug where the quick search
> menupopup doesn't properly check the right search mode when you first start
> up.  It always showed Subject or Sender. 

Bug 279855; I WFM'd it before I saw this comment.

Comment 9

[Alex Ihrig [:Thunderbird_Mail_DE]]

Attachment: Screenshot demonstrating the problem

Have a look at the attached screenshot. The buttons text is without any space behind the buttons icons. In Firefox this is different. Maybe some CSS are missing in Thunderbirds standard theme?

Comment 10

[Alex Ihrig [:Thunderbird_Mail_DE]]

There is one more Bug in Thunderbird findbar implementation.

The entity "find.label" exists in findbar.dtd and in messenger.dtd.
- find.label in findbar.dtd is "Find:" with the ":"
- find.label in messenger.dtd is "Find" withOUT the ":"

"Find" in messenger.dtd is used for the menu item "Find" in the menu "Edit". It must not have the ":". This is displayed correct.

Because of the same entity name, in 3-pane-window the Findbar has only "Find" (instead of "Find:" before the textbox).

We should fix this by renaming "find.label" in messenger.dtd. Findbar.dtd must not be changed because of Firefox.

BTW: messenger.dtd includes an entity "searchMenu.label" which seems not to be used in any XUL file. Maybe we could use this entity for the menu item?

Comment 11

[Alex Ihrig [:Thunderbird_Mail_DE]]

Attachment: [checked in]proposed fix for find.label duplication

(In reply to comment #10)
Best solution for l10n teams would be to change the entity names in messenger.dtd / mailWindowOverlay.xul.

Comment 12

[Alex Ihrig [:Thunderbird_Mail_DE]]

(In reply to comment #5)
> +  // If the find bar is visible and we just loaded a new message, re-run 
> +  // the find command. This means the new message will get highlighted and
> +  // we'll scroll to the first word in the message that matches the find text.
> +  if (gFindBar.isFindBarVisible())
> +    gFindBar.find();
> 
> 
> I guess we'll see how this feels. This will happen in any view, right?
> 

The findbar searches the mail start page, too. Only a "cosmetic" problem.

Comment 13

[Scott MacGregor]

Comment on attachment 214113 [details] [diff] [review]
[checked in]proposed fix for find.label duplication

Thanks Alex. I'll check this in.

Comment 14

[Scott MacGregor]

re-closing this bug. Let's file a new bug for finding out why the find bar isn't getting padding around the buttons. I've already checked in the label duplication patch into the trunk and the branch. 

Comment 15

[Alex Ihrig [:Thunderbird_Mail_DE]]

(In reply to comment #14)
> Let's file a new bug for finding out why the find bar
> isn't getting padding around the buttons.  

https://bugzilla.mozilla.org/show_bug.cgi?id=330402

Comment 16

[Miles Libbey]

*** Bug 238060 has been marked as a duplicate of this bug. ***

Comment 17

[Alex Ihrig [:Thunderbird_Mail_DE]]

What about the findbar in the standalone message window?

Comment 18

[Scott MacGregor]

(In reply to comment #17)
> What about the findbar in the standalone message window?
> 

seems to be working ok for me Alexander. What problem are you having with it?

Comment 19

[Mike Cowperthwaite]

(In reply to comment #18)
> (In reply to comment #17)
> > What about the findbar in the standalone message window?
> 
> seems to be working ok for me Alexander. What problem are you having with it?

I wonder if Alexander is talking about the problem from bug 324194 comment 4.

Comment 20

[Ihar 'Philips' Filipau]

Is there any extension planned for Tb to "fix" the behaviour to proper default one with dialog box?

I do not see from the discussion that new behaviour is optional. Ff has extension "Retro Find". Is there anything similar but for Tb?

P.S. IMHO, the find tool bar is worst GUI decision of Ff. But it's IMHO. Good ol' Mozilla's typeahead w/o any tool bar + classical dialog box on ^F I think was proper compromise.

Comment 21

[Sylvain Pasche]

This conversion introduces a regression for which I opened bug 359684.

I would be happy to know if the lack of highlighting in that situation is intentional or really a bug.