279227 - Move find bar strings out of browser, #ifdef USE_FIND_BAR the find bar code that Thunderbird builds

Status: RESOLVED FIXED

(Toolkit :: Find Toolbar, defect)

Description

[Jeff Walden (gone starting June 8)]

The Find Bar uses entities from <chrome://browser/locale/browser.dtd>.  This is
bad.  They should be moved into a new DTD in tookit/components/typeaheadfind so
that applications other than Firefox that want the Find Bar can use it without
toolkit source code hacks.

Comment 1

[Jeff Walden (gone starting June 8)]

I need this to fix bug 260058 and bug 253334, and I have a decent patch for bug
260058 waiting in my tree for when this bug is fixed.

-->me

Comment 2

[Jeff Walden (gone starting June 8)]

Attachment: Move entities/strings into separate .dtd/.properties, #ifdef things with USE_FIND_BAR

This patch is, overall, pretty trivial.  The find bar-specific entities and
properties get split out into their own files in
toolkit/locales/en-US/chrome/global/findbar.*, and I change the various doctype
declarations, stringbundle URLs, and other such stuff in help, browser, and
viewsource as necessary.  This lets me get rid of bogus references to
chrome://browser/locale/browser.dtd in a couple places in toolkit code.  I also
add a few entities (entities *not* specific to the find bar) that need to be
added in help.dtd and viewSource.dtd.  Finally, while I'm tweaking stuff
anyways, I use some preprocessing to produce the correct code when the find bar
isn't being built (using DEFINES += -DUSE_FIND_BAR).  The preprocessing adds a
bunch of non-platform-specific ifdefs, which are bad, but it consolidates the
app-specific ifdefs to only two locations, making future changes easier when
Thunderbird behaves better.  The ifdefs also clearly delineate the switching
between the two methods for finding text.

I've tested this patch on otherwise clean builds of both Firefox (using View
Source, View Partial Source, Help, and the main browser itself) and Thunderbird
(View Message Source), and everything continues to work perfectly.

Comment 3

[Mike Connor [:mconnor]]

Comment on attachment 172358 [details] [diff] [review]
Move entities/strings into separate .dtd/.properties, #ifdef things with USE_FIND_BAR

backing up a step, this isn't strictly a dependency on toolkit.  If you include
the findbar.inc file, you have to have the right entities kicking around in the
DTD's your window uses.  If help.dtd has these entities, you're fine.

That being said, if the strings are going to be the same anyway, they should be
in a central location in order to reduce l10n load.  Moving on...


>Index: mozilla/browser/base/content/browser-doctype.inc
>===================================================================
>RCS file: /cvsroot/mozilla/browser/base/content/browser-doctype.inc,v
>retrieving revision 1.3
>diff -p -u -8 -r1.3 browser-doctype.inc
>--- mozilla/browser/base/content/browser-doctype.inc	30 Nov 2004 08:22:43 -0000	1.3
>+++ mozilla/browser/base/content/browser-doctype.inc	25 Jan 2005 15:45:07 -0000
>@@ -4,10 +4,12 @@
> <!ENTITY % browserDTD SYSTEM "chrome://browser/locale/browser.dtd" >
> %browserDTD;
> <!ENTITY % globalRegionDTD SYSTEM "chrome://global-region/locale/region.dtd">
> %globalRegionDTD;
> <!ENTITY % charsetDTD SYSTEM "chrome://global/locale/charsetOverlay.dtd" >
> %charsetDTD;
> <!ENTITY % findDTD SYSTEM "chrome://global/locale/finddialog.dtd" >
> %findDTD;

what do we use from finddialog.dtd?  I would think this is in the past and we
can remove this?

>+<!ENTITY % findBarDTD SYSTEM "chrome://global/locale/findbar.dtd" >
>+%findBarDTD;
> ]>
> 
>Index: mozilla/browser/base/content/browser-sets.inc
>===================================================================
>RCS file: /cvsroot/mozilla/browser/base/content/browser-sets.inc,v
>retrieving revision 1.32
>diff -p -u -8 -r1.32 browser-sets.inc
>--- mozilla/browser/base/content/browser-sets.inc	24 Jan 2005 23:40:03 -0000	1.32
>+++ mozilla/browser/base/content/browser-sets.inc	25 Jan 2005 15:45:07 -0000
>@@ -40,17 +40,17 @@
> #define XP_GNOME 1
> #endif
> #endif
> 
>   <stringbundleset id="stringbundleset"> 
>     <stringbundle id="bundle_brand" src="chrome://global/locale/brand.properties"/>
>     <stringbundle id="bundle_shell" src="chrome://browser/locale/shellservice.properties"/>
>     <stringbundle id="bundle_browser" src="chrome://browser/locale/browser.properties"/>
>-    <stringbundle id="bundle_findBar" src="chrome://browser/locale/browser.properties"/>
>+    <stringbundle id="bundle_findBar" src="chrome://global/locale/findbar.properties"/>
>     <stringbundle id="bundle_browser_region" src="chrome://browser-region/locale/region.properties"/>
>   </stringbundleset>
> 
>   <commandset commandupdater="true"
>               events="focus"
>               oncommandupdate="goUpdateGlobalEditMenuItems()"/>
>   <commandset commandupdater="true"
>               events="select"

>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/components/help/Makefile.in,v
>retrieving revision 1.5
>diff -p -u -8 -r1.5 Makefile.in
>--- mozilla/toolkit/components/help/Makefile.in	1 Dec 2004 18:58:09 -0000	1.5
>+++ mozilla/toolkit/components/help/Makefile.in	25 Jan 2005 15:45:07 -0000
>@@ -15,17 +15,17 @@
> #
> # The Initial Developer of the Original Code is
> # Netscape Communications Corporation.
> # Portions created by the Initial Developer are Copyright (C) 2002
> # the Initial Developer. All Rights Reserved.
> #
> # Contributor(s):
> #  R.J. Keller <[e-mail]>
>-#  Jeff Walden <[e-mail]>
>+#  Jeff Walden <[e-mail]>
> #
> # Alternatively, the contents of this file may be used under the terms of
> # either the GNU General Public License Version 2 or later (the "GPL"), or
> # the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
> # in which case the provisions of the GPL or the LGPL are applicable instead
> # of those above. If you wish to allow use of your version of this file only
> # under the terms of either the GPL or the LGPL, and not to allow others to
> # use your version of this file under the terms of the MPL, indicate your
>@@ -42,8 +42,12 @@ srcdir		= @srcdir@
> VPATH		= @srcdir@
> 
> DIRS += helpOnHelp
> 
> include $(DEPTH)/config/autoconf.mk
> 
> include $(topsrcdir)/config/rules.mk
> 
>+#XXXjwalden hack until Thunderbird uses toolkit's typeaheadfind
>+ifdef MOZ_PHOENIX
>+DEFINES += -DUSE_FIND_BAR
>+endif

how about, instead, #ifndef MOZ_THUNDERBIRD?

The define should be USE_FIND_TOOLBAR, just for the sake of obviousness.

>Index: mozilla/toolkit/components/help/content/help.js
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/components/help/content/help.js,v
>retrieving revision 1.18
>diff -p -u -8 -r1.18 help.js
>--- mozilla/toolkit/components/help/content/help.js	26 Dec 2004 18:23:41 -0000	1.18
>+++ mozilla/toolkit/components/help/content/help.js	25 Jan 2005 15:45:07 -0000
>@@ -121,17 +121,19 @@ function displayTopic(topic) {
> function init() {
>     // Cache panel references.
>     helpSearchPanel = document.getElementById("help-search-panel");
>     helpTocPanel = document.getElementById("help-toc-panel");
>     helpIndexPanel = document.getElementById("help-index-panel");
>     helpGlossaryPanel = document.getElementById("help-glossary-panel");
>     helpBrowser = document.getElementById("help-content");
>     
>+#ifdef USE_FIND_BAR
>     initFindBar();
>+#endif
> 
>     // Get the content pack, base URL, and help topic
>     var helpTopic = defaultTopic;
>     if ("arguments" in window
>             && window.arguments[0]
>             instanceof Components.interfaces.nsIDialogParamBlock) {
>         helpFileURI = window.arguments[0].GetString(0);
>         // trailing "/" included.
>Index: mozilla/toolkit/components/help/content/help.xul
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/components/help/content/help.xul,v
>retrieving revision 1.16
>diff -p -u -8 -r1.16 help.xul
>--- mozilla/toolkit/components/help/content/help.xul	21 Jan 2005 08:02:19 -0000	1.16
>+++ mozilla/toolkit/components/help/content/help.xul	25 Jan 2005 15:45:07 -0000
>@@ -17,16 +17,17 @@
> # The Initial Developer of the Original Code is
> # Ian Oeschger.
> # Portions created by the Initial Developer are Copyright (C) 2003-2004
> # the Initial Developer. All Rights Reserved.
> #
> # Contributor(s):
> #   R.J. Keller <[e-mail]>
> #   Steffen Wilberg <[e-mail]>
>+#   Jeff Walden <[e-mail]>
> #
> # Alternatively, the contents of this file may be used under the terms of
> # either the GNU General Public License Version 2 or later (the "GPL"), or
> # the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
> # in which case the provisions of the GPL or the LGPL are applicable instead
> # of those above. If you wish to allow use of your version of this file only
> # under the terms of either the GPL or the LGPL, and not to allow others to
> # use your version of this file under the terms of the MPL, indicate your
>@@ -34,47 +35,61 @@
> # and other provisions required by the LGPL or the GPL. If you do not delete
> # the provisions above, a recipient may use your version of this file under
> # the terms of any one of the MPL, the GPL or the LGPL.
> #
> # ***** END LICENSE BLOCK *****
> 
> <?xml-stylesheet href="chrome://help/skin/" type="text/css"?>
> <?xml-stylesheet href="chrome://help/skin/sidebar.css" type="text/css"?>
>+#ifdef USE_FIND_BAR
> <?xml-stylesheet href="chrome://global/skin/findBar.css" type="text/css"?>
>+#endif
> 
> <?xul-overlay href="chrome://help/content/helpContextOverlay.xul"?>
> <!DOCTYPE window [
>     <!ENTITY % brandDTD SYSTEM "chrome://global/locale/brand.dtd">
>     %brandDTD;
>     <!ENTITY % helpDTD SYSTEM "chrome://help/locale/help.dtd">
>     %helpDTD;
>-    <!ENTITY % browserDTD SYSTEM "chrome://browser/locale/browser.dtd" >
>-    %browserDTD;
>+#ifdef USE_FIND_BAR
>+    <!ENTITY % findBarDTD SYSTEM "chrome://global/locale/findbar.dtd" >
>+    %findBarDTD;


You seem to have forgotten an #else here.  Shouldn't the following be defined
if we're not using FT?

>     <!ENTITY % findDTD SYSTEM "chrome://global/locale/finddialog.dtd" >
>     %findDTD;
>+#endif

>Index: mozilla/toolkit/components/help/locale/en-US/help.dtd
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/components/help/locale/en-US/help.dtd,v
>retrieving revision 1.8
>diff -p -u -8 -r1.8 help.dtd
>--- mozilla/toolkit/components/help/locale/en-US/help.dtd	21 Jan 2005 08:02:19 -0000	1.8
>+++ mozilla/toolkit/components/help/locale/en-US/help.dtd	25 Jan 2005 15:45:07 -0000
>@@ -1,15 +1,16 @@
> <!ENTITY printCmd.commandkey            "p">
> 
> <!ENTITY viewCustomizeToolbar.label     "Customize...">
> <!ENTITY viewCustomizeToolbar.accesskey "C">
> 
> <!ENTITY findOnCmd.commandkey           "F">
> <!ENTITY findAgainCmd.commandkey        "G">
>+<!ENTITY findAgainCmd.commandkey2       "VK_F3">
> 
> <!ENTITY backButton.label               "Back">
> <!ENTITY backButton.accesskey           "B">
> <!ENTITY backButton.tooltip             "Go back one page">
> <!ENTITY forwardButton.label            "Forward">
> <!ENTITY forwardButton.accesskey        "F">
> <!ENTITY forwardButton.tooltip          "Go forward one page">
> <!ENTITY goBackCmd.commandkey           "[">
>Index: mozilla/toolkit/components/viewsource/Makefile.in
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/components/viewsource/Makefile.in,v
>retrieving revision 1.1
>diff -p -u -8 -r1.1 Makefile.in
>--- mozilla/toolkit/components/viewsource/Makefile.in	1 Mar 2003 08:07:03 -0000	1.1
>+++ mozilla/toolkit/components/viewsource/Makefile.in	25 Jan 2005 15:45:07 -0000
>@@ -1,31 +1,49 @@
>+# ***** BEGIN LICENSE BLOCK *****
>+# Version: MPL 1.1/GPL 2.0/LGPL 2.1
> #
>-# The contents of this file are subject to the Netscape Public
>-# License Version 1.1 (the "License"); you may not use this file
>-# except in compliance with the License. You may obtain a copy of
>-# the License at http://www.mozilla.org/NPL/
>-#
>-# Software distributed under the License is distributed on an "AS
>-# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
>-# implied. See the License for the specific language governing
>-# rights and limitations under the License.
>-#
>-# The Original Code is mozilla.org code.
>-#
>-# The Initial Developer of the Original Code is Netscape
>-# Communications Corporation.  Portions created by Netscape are
>-# Copyright (C) 1998 Netscape Communications Corporation. All
>-# Rights Reserved.
>+# The contents of this file are subject to the Mozilla Public License Version
>+# 1.1 (the "License"); you may not use this file except in compliance with
>+# the License. You may obtain a copy of the License at
>+# http://www.mozilla.org/MPL/
>+#
>+# Software distributed under the License is distributed on an "AS IS" basis,
>+# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
>+# for the specific language governing rights and limitations under the
>+# License.
>+#
>+# The Original Code is Toolkit View Source.

mozilla.org Code is still the right answer here.

also, maybe ask Gerv about whether relicensing this is "right" or not.	You
also only added one check, that doesn't make you the initial developer.

>+#
>+# The Initial Developer of the Original Code is
>+# Jeff Walden <[e-mail]>.
>+# Portions created by the Initial Developer are Copyright (C) 2005
>+# the Initial Developer. All Rights Reserved.
>+#
>+# Contributor(s):
>+#
>+# Alternatively, the contents of this file may be used under the terms of
>+# either the GNU General Public License Version 2 or later (the "GPL"), or
>+# the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
>+# in which case the provisions of the GPL or the LGPL are applicable instead
>+# of those above. If you wish to allow use of your version of this file only
>+# under the terms of either the GPL or the LGPL, and not to allow others to
>+# use your version of this file under the terms of the MPL, indicate your
>+# decision by deleting the provisions above and replace them with the notice
>+# and other provisions required by the GPL or the LGPL. If you do not delete
>+# the provisions above, a recipient may use your version of this file under
>+# the terms of any one of the MPL, the GPL or the LGPL.
> #
>-# Contributor(s): 
>-#
>-
>+# ***** END LICENSE BLOCK *****
> 
> DEPTH     = ../../..
> topsrcdir = @top_srcdir@
> srcdir    = @srcdir@
> VPATH     = @srcdir@
> 
> include $(DEPTH)/config/autoconf.mk
> 
> include $(topsrcdir)/config/rules.mk
> 

#ifndef MOZ_THUNDERBIRD here please

>+#XXXjwalden hack until Thunderbird uses toolkit's typeaheadfind
>+ifdef MOZ_PHOENIX
>+DEFINES += -DUSE_FIND_BAR
>+endif


>Index: mozilla/toolkit/components/viewsource/content/viewPartialSource.xul
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/components/viewsource/content/viewPartialSource.xul,v
>retrieving revision 1.13
>diff -p -u -8 -r1.13 viewPartialSource.xul
>--- mozilla/toolkit/components/viewsource/content/viewPartialSource.xul	25 Jan 2005 00:18:58 -0000	1.13
>+++ mozilla/toolkit/components/viewsource/content/viewPartialSource.xul	25 Jan 2005 15:45:07 -0000
> <!DOCTYPE window [
> <!ENTITY % brandDTD SYSTEM "chrome://global/locale/brand.dtd" >
> %brandDTD;
> <!ENTITY % sourceDTD SYSTEM "chrome://global/locale/viewSource.dtd" >
> %sourceDTD;
>-<!ENTITY % browserDTD SYSTEM "chrome://browser/locale/browser.dtd" >
>-%browserDTD;
>+<!ENTITY % findBarDTD SYSTEM "chrome://global/locale/findbar.dtd" >
>+%findBarDTD;

next problem.  Doesn't thunderbird use this?  If not, ok.

>Index: mozilla/toolkit/locales/en-US/chrome/global/findbar.properties
>===================================================================
>RCS file: mozilla/toolkit/locales/en-US/chrome/global/findbar.properties
>diff -N mozilla/toolkit/locales/en-US/chrome/global/findbar.properties
>--- /dev/null	1 Jan 1970 00:00:00 -0000
>+++ mozilla/toolkit/locales/en-US/chrome/global/findbar.properties	25 Jan 2005 15:45:07 -0000
>@@ -0,0 +1,4 @@
>+# strings used by the Find bar, split from browser.properties
>+NotFound=Phrase not found
>+WrappedToTop=Reached end of page, continued from top
>+WrappedToBottom=Reached top of page, continued from bottom

where's the bit where these get removed from browser.propertie?

Comment 4

[Darin Fisher]

Please try to ensure that "#ifdef USE_FIND_BAR" won't be something that has to
be undone when we move to a shared runtime (XULRunner) for Firefox and Tbird. 
Thanks!

NOTE: I understand that some of the code under mozilla/toolkit should probably
not be included with XULRunner, and I haven't checked to see if that applies to
the code you are #ifdef'ing.

Comment 5

[Jeff Walden (gone starting June 8)]

Attachment: Patch, v2

> > <!ENTITY % findDTD SYSTEM "chrome://global/locale/finddialog.dtd" >
> > %findDTD;
> 
> what do we use from finddialog.dtd?  I would think this is in the past and we

> can remove this?

Pseudo-past.  There are only two entities in there that are used in the find
bar, so I copied them over into findbar.dtd.  (They're also used in the find
dialog, so I couldn't just move them wholesale.)  This makes findbar.dtd
incompatible with finddialog.dtd, which makes sense as an app won't use both
the find bar and find dialogs.

> >+#XXXjwalden hack until Thunderbird uses toolkit's typeaheadfind
> >+ifdef MOZ_PHOENIX
> >+DEFINES += -DUSE_FIND_BAR
> >+endif
> 
> how about, instead, #ifndef MOZ_THUNDERBIRD?
> 
> The define should be USE_FIND_TOOLBAR, just for the sake of obviousness.

Done.

> >+#ifdef USE_FIND_BAR
> >+	<!ENTITY % findBarDTD SYSTEM "chrome://global/locale/findbar.dtd" >
> >+	%findBarDTD;
> 
> 
> You seem to have forgotten an #else here.  Shouldn't the following be defined

> if we're not using FT?

A bunch of LXR searches have confirmed that finddialog.dtd actually isn't
needed outside of finddialog.xul, so it's been removed in all the code I've
touched (leaving only the one real reference in finddialog.xul remaining).

> mozilla.org Code is still the right answer here.
> 
> also, maybe ask Gerv about whether relicensing this is "right" or not. You
> also only added one check, that doesn't make you the initial developer.

It was supposed to be a drive-by relicensing.  There's nothing original in that
file, so I just rewrote it from scratch and gave it the new license.  It
doesn't really matter, tho, so I grabbed the old version and made the changes
there.

> #ifndef MOZ_THUNDERBIRD here please
> 
> >+#XXXjwalden hack until Thunderbird uses toolkit's typeaheadfind
> >+ifdef MOZ_PHOENIX
> >+DEFINES += -DUSE_FIND_BAR
> >+endif

Also done.

> >Index: mozilla/toolkit/components/viewsource/content/viewPartialSource.xul
> next problem.  Doesn't thunderbird use this?	If not, ok.

I originally thought no, but then I started doing some checking.  It looks like
there *might* be a codepath that Thunderbird uses to get this, but I couldn't
find how it would actually be accessed.  (As it doesn't appear to have been
exercised yet by the absence of a bug in bmo, I felt relatively safe ignoring
it.)  I added back old find dialog code encased in ifdefs just to be on the
safe side.

> >@@ -0,0 +1,4 @@
> >+# strings used by the Find bar, split from browser.properties
> >+NotFound=Phrase not found
> >+WrappedToTop=Reached end of page, continued from top
> >+WrappedToBottom=Reached top of page, continued from bottom
> 
> where's the bit where these get removed from browser.propertie?

It was forgotten in the original, but it's been done now.

I did some more builds and tested them, both in the configurations presented in
the patch and in opposite ones.  When I tested with Firefox, Help, View Source,
and View Partial Source performed correctly with USE_FIND_TOOLBAR either set or
unset.	Thunderbird's View Message Source also worked fine with
USE_FIND_TOOLBAR disabled.

Re comment 4, as discussed on IRC, this patch should be okay for now.  I'd fix
Thunderbird if I knew how, but I don't, so I'm cutting the app ifdefs down in
number as much as I can and consolidating as much as possible.	They should be
easy to remove or leave and ignore (assuming the ifndef MOZ_THUNDERBIRD around
DEFINES is removed) when Thunderbird's patched to use typeaheadfind.

Comment 6

[Jeff Walden (gone starting June 8)]

Attachment: Previous patch, unbitrotted

Incidentally, I'm pretty sure that's the fastest I've ever seen a patch bitrot.

Comment 7

[Jeff Walden (gone starting June 8)]

Comment on attachment 172512 [details] [diff] [review]
Previous patch, unbitrotted

Moving review to someone who may have more time in the next couple weeks...

Comment 8

[Jeff Walden (gone starting June 8)]

Attachment: Previous patch, unbitrotted again

Comment 9

[Darin Fisher]

who owns the find bar anyways?  should you get the owner if any to review this
first?

Comment 10

[Asaf Romano (gone)]

blake(In reply to comment #9)
> who owns the find bar anyways?  should you get the owner if any to review this
> first?

Blake Ross, I asuume it answers your second question as well.

Comment 11

[Darin Fisher]

cc'ing blake then.  i also sent him a direct email.

Comment 12

[Jeff Walden [:Waldo]]

Comment on attachment 175189 [details] [diff] [review]
Previous patch, unbitrotted again

This is almost certainly (trivially) bitrotted by the options window landing. 
I haven't yet been able to fix it because of bug 283738 (hacking is easier in
Linux, and I'm not updating my tree and rebuilding until I know I'll have a
functional build with prefs available, should I need them).  However, if the
situation requires it, I could always move the patch over into Windows to fix
it.

For now, tho, I'm hoping bug 283738 will get fixed before I need to unbitrot
the patch.

Comment 13

[Jeff Walden (gone starting June 8)]

Attachment: Previous patch, unbitrotted thrice

Comment 14

[Benjamin Smedberg]

Are the toolkit findbar and the seamonkey typeahead mutually exclusive? I am
seriously opposed to new #ifdefs of this sort. Let's fix it right, or not mess
with it at all.

Comment 15

[Darin Fisher]

Yeah, why move findbar into toolkit if it isn't used by all toolkit consumers?

Comment 16

[Jeff Walden (gone starting June 8)]

(In reply to comment #14)
> Are the toolkit findbar and the seamonkey typeahead mutually exclusive?

I've confirmed with mconnor that that's true.

(In reply to comment #15)
> Yeah, why move findbar into toolkit if it isn't used by all toolkit consumers?

The find bar is already in toolkit because that's where Blake put it.  It's far
better than a find dialog in the situations I can imagine, so I don't see why it
wouldn't be in toolkit.  After all, it's not limited to FAYT-style behavior - it
*can* be used for regular finding separately from find-as-you-type style finds.

I've sent an email to mscott asking him whether he could move Thunderbird over
to using toolkit's typeaheadfind, so hopefully something will get done there
(because I definitely can't do it), but I still hope this might be allowed into
source.

Comment 17

[Jeff Walden (gone starting June 8)]

Comment on attachment 175825 [details] [diff] [review]
Previous patch, unbitrotted thrice

Seeing objections to the ifdefs here with little chance of a change of heart, I
plan to redo this patch without them.  Getting Help in Thunderbird will simply
have to wait until Thunderbird uses toolkit's typeaheadfind, whenever that may
be; the problem's out of my hands now.

The new patch should be similar to this one and won't require much work, so I'm
going to try and finish it (and hopefully get it reviewed and checked into
source) before the next freeze.

Comment 18

[Asaf Romano (gone)]

(In reply to comment #17)
> (From update of attachment 175825 [details] [diff] [review] [edit])
> Seeing objections to the ifdefs here with little chance of a change of heart, I
> plan to redo this patch without them.  Getting Help in Thunderbird will simply
> have to wait until Thunderbird uses toolkit's typeaheadfind, whenever that may
> be; the problem's out of my hands now.

mscott: is it even planed? (In general, not just for help).

Comment 19

[Scott MacGregor]

mano, we don't have plans to add type as you find support in thunderbird. It can
be added as an extension. 

Comment 20

[Jeff Walden (gone starting June 8)]

Attachment: Move findbar strings into toolkit, don't introduce any new app #ifdefs

The patch basically does the same thing as previously posted patches, but it
doesn't introduce ifdefs for Thunderbird support.  It removes some browser
dependencies by moving the relevant code into toolkit.

viewSource.xul still has some ifdefs in it because Thunderbird uses it.  I've
converted the ifdefs by using USE_FIND_TOOLBAR and defining it in
viewSource.xul if we aren't building Thunderbird.  Thus, *only* Thunderbird
gets non-find bar code and everyone else gets good code.  It's a little cleaner
than what's currently there, although it's still hackish.  (I didn't use this
for viewPartialSource.xul because viewPartialSource.* isn't bundled in
Thunderbird builds, as determined by downloading a build, extracting
everything, and finding only viewSource.* and not viewPartialSource.*.)

I also made one unrelated change to remove a line from help.xul referencing
chrome://browser/content/sessionHistoryUI.js; LXR will show this file no longer
exists.

Comment 21

[Asaf Romano (gone)]

considering comment 19, you do want the #ifdefs.

Comment 22

[Asaf Romano (gone)]

(In reply to comment #19)
> mano, we don't have plans to add type as you find support in thunderbird. It can
> be added as an extension. 

mscott: to be clear, i've also reffered to the findtoolbar in general, not just
to TAF (i.e "Find in this page" mode).

Comment 23

[Jeff Walden (gone starting June 8)]

Attachment: Previous patch, unbitrotted

This patch, unlike the last one as of very recently, will not produce conflicts
in browser-sets.inc.

(In reply to comment #21)
I want them, true, but judging by the action happening here I'm not getting
them.  Not using #ifdefs may not be optimal from my point of view, but it does
get rid of a bunch of browser dependencies in toolkit.	This makes things
better for xulrunner users even if Thunderbird doesn't benefit.

(In reply to comment #22)
> (In reply to comment #19)
> > mano, we don't have plans to add type as you find support in thunderbird.
It
> > can be added as an extension. 
> 
> mscott: to be clear, i've also reffered to the findtoolbar in general, not
> just to TAF (i.e "Find in this page" mode).

Indeed; just indiscriminately finding while typing would conflict with a ton of
different existing keyboard shortcuts in Thunderbird (and thus using it makes
little sense).	However, the find bar is considerably better than a find
dialog.  The find bar's already used for old-style finds when accessed via
Ctrl+F in Firefox, and there's no reason the same couldn't be done in
Thunderbird.  Help, viewsource, and the main window could use FAYT wrapped
around old-style menu- or keyboard-based access, which would be perfectly
acceptable from a toolkit POV.

Comment 24

[Jeff Walden (gone starting June 8)]

Patch checked in, marking FIXED, and tweaking summary to reflect the slight
change in the bug.  On to the next bug...

http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&date=explicit&mindate=1112159640&maxdate=2005-03-29+21%3A29%3A00

Comment 25

[Michiel van Leeuwen (email: mvl+moz@)]

Too bad the bug isn't actually fixed. We are not closer to one toolkit.

Comment 26

[Darin Fisher]

(In reply to comment #25)
> Too bad the bug isn't actually fixed. We are not closer to one toolkit.

small, small, catch monkey.  this is progress.  we'll get there :-)