623889 - Move the FAYT preferences to their own panel under Advanced.

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Edmund Wong (:ewong)]

Bug #568283 adds a checkbox to the Advanced->Keyboard Navigation.  As suggested
by bug #568283 comment #1,  FAYT is to be moved to its own panel under
Advanced.

Comment 1

[Edmund Wong (:ewong)]

Picking it up for a spin.

Comment 2

[Edmund Wong (:ewong)]

Attachment: Moved FAYT to its own panel under Advanced. (v1)

Comment 3

[Ian Neal]

Comment on attachment 501979 [details] [diff] [review]
Moved FAYT to its own panel under Advanced. (v1)

As suggested in bug 568283 we need somewhere for accessibility.browsewithcaret to live, so if it is in this pref panel it can not be called Find As You Type, but it probably makes more sense under Keyboard Navigation (as it is to do with using cursor keys I believe).
Anyway if we stick with Find As You Type then I would prefer a different filename to pref-fayt and for the entities / ids, perhaps pref-findasyoutype and findAsYouType?

>+++ b/suite/common/pref/pref-fayt.js
>+ * Contributor(s): Edmund Wong <[e-mail]>
Nit: the name of the contributor usually goes on the line after, so "* Contributor(s)" on one line and the name(s) on the following line(s).

>+function Startup()
>+{
>+  var prefAutostart = document.getElementById("accessibility.typeaheadfind.autostart");
>+  SetLinksOnlyEnabled(prefAutostart.value);
>+}
>+
>+function SetLinksOnlyEnabled(aEnable)
>+{
>+  EnableElementById("findAsYouTypeAutoWhat", aEnable, false);
>+}
I presume these were copied out of pref-keynav.js but there are no changes to that file in this diff!

>+++ b/suite/common/pref/pref-fayt.xul
>+   - Contributor(s): Edmund Wong <[e-mail]> 
Nit: see above

>+      <preference id="accessibility.tabfocus"
>+                  name="accessibility.tabfocus"
>+                  type="int"/>
A copy-and-paste mistake? This is in keynav still.

>+++ b/suite/common/pref/pref-keynav.xul
>@@ -47,25 +47,9 @@
>       <preference id="accessibility.tabfocus"
>                   name="accessibility.tabfocus"
>                   type="int"/>
See.

>+++ b/suite/locales/en-US/chrome/common/pref/pref-fayt.dtd
>@@ -0,0 +1,17 @@
>+<!ENTITY pref.fayt.title "Find As You Type">
>+<!ENTITY findAsYouTypeBehavior.label "Find As You Type">
>+<!ENTITY findAsYouTypeTip.label "Tip: To manually start Find As You Type, type / to find text or ' to find links, followed by the text you want to find.">
>+<!ENTITY findAsYouTypeTimeout.label "Clear the current search after a few seconds of inactivity">
>+<!ENTITY findAsYouTypeTimeout.accesskey "C">
>+<!ENTITY findAsYouTypeSound.label "Play a sound when typed text isn't found">
>+<!ENTITY findAsYouTypeSound.accesskey "P">
>+<!ENTITY findAsYouTypeEnableAuto.label "Find automatically when typing within a web page:">
>+<!ENTITY findAsYouTypeEnableAuto.accesskey "F">
>+<!ENTITY findAsYouTypeAutoText.label "Any text in the page">
>+<!ENTITY findAsYouTypeAutoText.accesskey "A">
>+<!ENTITY findAsYouTypeAutoLinks.label "Links only">
>+<!ENTITY findAsYouTypeAutoLinks.accesskey "o">
You could use "L" here now as there is nothing else using it in this pref pane.

r- for the moment

Comment 4

[Edmund Wong (:ewong)]

(In reply to comment #3)
> Comment on attachment 501979 [details] [diff] [review]
> Moved FAYT to its own panel under Advanced. (v1)
> 
> As suggested in bug 568283 we need somewhere for accessibility.browsewithcaret
> to live, so if it is in this pref panel it can not be called Find As You Type,
> but it probably makes more sense under Keyboard Navigation (as it is to do with
> using cursor keys I believe).

Actually, my intention was for this bug to move the FAYT options into its own
panel under Advanced, and then for that bug 568283, put the 
accessibiility.browsewithcaret to be in the Keyboard Navigation panel along
with the other Keyboard Navigation options.

Comment 5

[Ian Neal]

(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 501979 [details] [diff] [review]
> > Moved FAYT to its own panel under Advanced. (v1)
> > 
> > As suggested in bug 568283 we need somewhere for accessibility.browsewithcaret
> > to live, so if it is in this pref panel it can not be called Find As You Type,
> > but it probably makes more sense under Keyboard Navigation (as it is to do with
> > using cursor keys I believe).
> 
> Actually, my intention was for this bug to move the FAYT options into its own
> panel under Advanced, and then for that bug 568283, put the 
> accessibiility.browsewithcaret to be in the Keyboard Navigation panel along
> with the other Keyboard Navigation options.

Yes, that is what I said too.

Comment 6

[Philip Chee]

how about pref-typeaheadfind.{xul|js} ?

Comment 7

[Edmund Wong (:ewong)]

Attachment: Moved the FAYT preferences to its own panel under Advanced. (v2)

Comment 8

[Ian Neal]

Comment on attachment 503437 [details] [diff] [review]
Moved the FAYT preferences to its own panel under Advanced. (v2)

this is identical to the previous one!

Comment 9

[Edmund Wong (:ewong)]

Attachment: Moved FAYT to its own panel under Advanced. (v3)

Comment 10

[Ian Neal]

Comment on attachment 504219 [details] [diff] [review]
Moved FAYT to its own panel under Advanced. (v3)

>+++ b/suite/common/pref/pref-findasyoutype.js

>+ * Contributor(s): 
>+ *                 Edmund Wong <[e-mail]>
Nit: The first letter of the name of the contributor usually lines up with the n of Contributor(s).

>+++ b/suite/common/pref/pref-findasyoutype.xul
>+   - Contributor(s): 
>+                    Edmund Wong <[e-mail]>
Nit: The first letter of the name of the contributor usually lines up with the n of Contributor(s).

+++ b/suite/common/pref/preferences.xul
+          <treeitem id="findAsYouTypeItem"
+                    label="&findAsYouType.label;"
+                    prefpane="findAsYouType_pane"
+                    helpTopic="advanced_pref_findAsYouType"
+                    url="chrome://communicator/content/pref/pref-findAsYouType.xul"/>

+++ b/suite/locales/en-US/chrome/common/pref/preferences.dtd
@@ -34,5 +34,6 @@
 <!ENTITY policies.label "Security Policies">
 <!ENTITY mousewheel.label "Mouse Wheel">
 <!ENTITY scriptsAndWindows.label "Scripts &amp; Plugins">
+<!ENTITY findasyoutype.label "Find As You Type">

Your entity doesn't match between the two files and the url is incorrect.

You no longer appear to be adding the new dtd file for the new pref panel into suite/locales/jar.mn.

r- most of these should have been picked up from just trying to test the patch, please test before submitting.

Comment 11

[Edmund Wong (:ewong)]

(In reply to comment #10)
> +++ b/suite/common/pref/preferences.xul
> +          <treeitem id="findAsYouTypeItem"
> +                    label="&findAsYouType.label;"
> +                    prefpane="findAsYouType_pane"
> +                    helpTopic="advanced_pref_findAsYouType"
> +                   
> url="chrome://communicator/content/pref/pref-findAsYouType.xul"/>
> 
> +++ b/suite/locales/en-US/chrome/common/pref/preferences.dtd
> @@ -34,5 +34,6 @@
>  <!ENTITY policies.label "Security Policies">
>  <!ENTITY mousewheel.label "Mouse Wheel">
>  <!ENTITY scriptsAndWindows.label "Scripts &amp; Plugins">
> +<!ENTITY findasyoutype.label "Find As You Type">
> 
> Your entity doesn't match between the two files and the url is incorrect.
> 
Can you point out what's wrong with the url?

Comment 12

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #11)
> Can you point out what's wrong with the url?

It's not the same as the actual one. Works on Windows but not on filesystems that take case into account (Unix/Linux, probably also Mac).

Comment 13

[Ian Neal]

(In reply to comment #11)
> (In reply to comment #10)
> > +++ b/suite/common/pref/preferences.xul
> > +          <treeitem id="findAsYouTypeItem"
> > +                    label="&findAsYouType.label;"
> > +                    prefpane="findAsYouType_pane"
> > +                    helpTopic="advanced_pref_findAsYouType"
> > +                   
> > url="chrome://communicator/content/pref/pref-findAsYouType.xul"/>
> > 
> > +++ b/suite/locales/en-US/chrome/common/pref/preferences.dtd
> > @@ -34,5 +34,6 @@
> >  <!ENTITY policies.label "Security Policies">
> >  <!ENTITY mousewheel.label "Mouse Wheel">
> >  <!ENTITY scriptsAndWindows.label "Scripts &amp; Plugins">
> > +<!ENTITY findasyoutype.label "Find As You Type">
> > 
> > Your entity doesn't match between the two files and the url is incorrect.
> > 
> Can you point out what's wrong with the url?

As Jens says, case sensitivity...

Comment 14

[Edmund Wong (:ewong)]

Attachment: Moved FAYT to its own panel under Advanced. (v4)

Comment 15

[Ian Neal]

Comment on attachment 510016 [details] [diff] [review]
Moved FAYT to its own panel under Advanced. (v4)

My only thought now is should we be copying the pref-keynav.* files to pref-findasyoutype.* files rather than creating new ones so as to keep history

Comment 16

[Ian Neal]

Attachment: Copy PAYT patch v5 [Checked in: Comment 18]

Using copy instead...

Comment 17

[Philip Chee]

> rather than creating new ones so as to keep history
Uh, what history? Other than the almost total rewrite during the move to the new <prefwindow>.

Comment 18

[Ian Neal]

Comment on attachment 510365 [details] [diff] [review]
Copy PAYT patch v5 [Checked in: Comment 18]

http://hg.mozilla.org/comm-central/rev/d7006d9c3c71

Comment 19

[Ian Neal]

I presume you're doing the browsewithcaret patch next (bug 568283) and then you could combine the help patches into one (bug 623901 and bug 623903)?

Comment 20

[Stefan [:stefanh]]

Comment on attachment 510365 [details] [diff] [review]
Copy PAYT patch v5 [Checked in: Comment 18]

 function Startup()
 {
   if (/Mac/.test(navigator.platform))
     document.getElementById("tabNavigationPrefs").setAttribute("hidden", true);
-
-  var prefAutostart = document.getElementById("accessibility.typeaheadfind.autostart");
-  SetLinksOnlyEnabled(prefAutostart.value);
 }

Note that this regressed Mac since we now show a blank panel. That is, please land the patch in bug 568283 asap ;-)