Closed Bug 623889 Opened 9 years ago Closed 9 years ago

Move the FAYT preferences to their own panel under Advanced.

Categories

(SeaMonkey :: Preferences, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ewong, Assigned: ewong)

References

Details

Attachments

(1 file, 4 obsolete files)

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.
Blocks: 568283
Picking it up for a spin.
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Blocks: 623901
Attachment #501979 - Flags: review?(iann_bugzilla)
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 <ewong@pw-wspx.org>
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 <ewong@pw-wspx.org> 
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
Attachment #501979 - Flags: review?(iann_bugzilla) → review-
Hardware: x86 → All
Version: unspecified → Trunk
(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.
(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.
how about pref-typeaheadfind.{xul|js} ?
Attachment #501979 - Attachment is obsolete: true
Attachment #503437 - Flags: review?(iann_bugzilla)
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!
Attachment #503437 - Flags: review?(iann_bugzilla) → review-
Attachment #503437 - Attachment is obsolete: true
Attachment #504219 - Flags: review?(iann_bugzilla)
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 <ewong@pw-wspx.org>
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 <ewong@pw-wspx.org>
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.
Attachment #504219 - Flags: review?(iann_bugzilla) → review-
(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?
(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).
(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...
Attachment #504219 - Attachment is obsolete: true
Attachment #510016 - Flags: review?(iann_bugzilla)
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
Attachment #510016 - Flags: review?(iann_bugzilla) → review+
Using copy instead...
Attachment #510365 - Flags: review+
Attachment #510365 - Flags: feedback?(ewong)
> 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>.
Attachment #510365 - Flags: feedback?(ewong) → feedback+
Attachment #510016 - Attachment is obsolete: true
Keywords: checkin-needed
Comment on attachment 510365 [details] [diff] [review]
Copy PAYT patch v5 [Checked in: Comment 18]

http://hg.mozilla.org/comm-central/rev/d7006d9c3c71
Attachment #510365 - Attachment description: Copy PAYT patch v5 → Copy PAYT patch v5 [Checked in: Comment 18]
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)?
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Blocks: 623903
No longer blocks: 623901
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 ;-)
You need to log in before you can comment on or make changes to this bug.