Add checkbox for accessibility.browsewithcaret (F7-by-default mode)

RESOLVED FIXED in seamonkey2.1b3

Status

SeaMonkey
Preferences
--
enhancement
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: InvisibleSmiley, Assigned: ewong)

Tracking

Trunk
seamonkey2.1b3
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 7 obsolete attachments)

3.91 KB, patch
ewong
: review+
ewong
: ui-review+
Details | Diff | Splinter Review
(Reporter)

Description

7 years ago
Spin-off from bug 465303: I suggest we add a checkbox for accessibility.browsewithcaret (F7-by-default mode) in Preferences, e.g. Advanced/Keyboard Navigation. FF has it under Advanced/General/Accessibility/Always use the cursor keys to navigate within pages.

Comment 1

7 years ago
Not much room there. Perhaps the Find-as-you-type groupbox can be moved into a panel of its own.

Comment 2

7 years ago
> Not much room there. Perhaps the Find-as-you-type groupbox can be moved into a
> panel of its own.
Only a suggestion: call it Advanced->Accessibility
And add |accessibility.browsewithcaret| to that panel.
(Assignee)

Updated

7 years ago
Depends on: 623889
(Assignee)

Updated

7 years ago
Assignee: nobody → ewong
Status: NEW → ASSIGNED
(Assignee)

Updated

7 years ago
Blocks: 623903
(Assignee)

Comment 3

7 years ago
Created attachment 513446 [details] [diff] [review]
Add checkbox to KeyNavigation for accessibility.browsewithcaret (F7-by-default mode)
Attachment #513446 - Flags: review?(iann_bugzilla)

Comment 4

7 years ago
> +<!ENTITY accessibilityBrowseWithCaret.label "Browse With Caret (F7-by-default mode)">

> +<!ENTITY browseWithCaretDesc.label "Always use the cursor keys to navigate within pages.">
Suggestions:

1. Enable keyboard navigation and selection within web pages using a visible caret.

2. Allows users to select arbitrary content with the keyboard and move through content as if inside a read-only editor. This allows copying arbitrary pieces of text to the clipboard. The F7 key toggles this feature on/off.

> +<!ENTITY browseWithCaret.label "Browse with Caret">
Repeating "Browse with Caret" doesn't convey any additional information.
Firefox:
<!ENTITY useCursorNavigation.label       "Always use the cursor keys to navigate within pages">

> +<!ENTITY browseWithCaret.accesskey "r">
Reference:
<https://developer.mozilla.org/en/XUL_Accesskey_FAQ_and_Policies#How_do_I_pick_an_accesskey_letter.3f>
Firefox:
<!ENTITY useCursorNavigation.accesskey   "c">
(Assignee)

Comment 5

7 years ago
Created attachment 513642 [details] [diff] [review]
Added checkbox for accessibility.browsewithcaret (F7-by-default mode)

Incl. fixes from comment #4
Attachment #513446 - Attachment is obsolete: true
Attachment #513642 - Flags: review?(iann_bugzilla)
Attachment #513446 - Flags: review?(iann_bugzilla)

Comment 6

7 years ago
(In reply to comment #5)
> Created attachment 513642 [details] [diff] [review]
> Added checkbox for accessibility.browsewithcaret (F7-by-default mode)
> 
> Incl. fixes from comment #4

Before I review it, just interested whether F7 is the key on all platforms so cc'ing stefanh for confirmation.

Comment 7

7 years ago
(In reply to comment #4)
> > +<!ENTITY browseWithCaret.accesskey "r">
> Reference:
> <https://developer.mozilla.org/en/XUL_Accesskey_FAQ_and_Policies#How_do_I_pick_an_accesskey_letter.3f>
> Firefox:
> <!ENTITY useCursorNavigation.accesskey   "c">
We have plenty of accesskeys to pick from on this pane, so I would say "A" was better.

Comment 8

7 years ago
(In reply to comment #6)
> Before I review it, just interested whether F7 is the key on all platforms so
> cc'ing stefanh for confirmation.

F7 is the key on Mac, yes.

Comment 9

7 years ago
Comment on attachment 513642 [details] [diff] [review]
Added checkbox for accessibility.browsewithcaret (F7-by-default mode)

>+++ b/suite/locales/en-US/chrome/common/pref/pref-keynav.dtd

>+<!ENTITY accessibilityBrowseWithCaret.label "Browse With Caret (F7-by-default mode)">
I'm not sure that we should have the (F7-by-default mode) bit here.

>+<!ENTITY browseWithCaretDesc.label "Allows users to select arbitrary content with the keyboard and move through content as if inside a read-only editor.  This allows copying arbitrary pieces of text to the clipboard.  The F7 key toggles this feature on/off.">
This sounds too wordy for a pref pane, more suited to help. I would prefer something like:
"Enable keyboard navigation and selection within web pages using a visible caret. The F7 key toggles this feature on/off."

>+<!ENTITY browseWithCaret.label "Always use the cursor keys to navigate within pages">
>+<!ENTITY browseWithCaret.accesskey "c">
"A" would be a better accesskey
Attachment #513642 - Flags: review?(iann_bugzilla) → review-
(Assignee)

Comment 10

7 years ago
Created attachment 513846 [details] [diff] [review]
Add checkbox to KeyNavigation for accessibility.browsewithcaret (F7-by-default mode) (v2)
Attachment #513642 - Attachment is obsolete: true
Attachment #513846 - Flags: review?(iann_bugzilla)

Updated

7 years ago
Attachment #513846 - Flags: review?(iann_bugzilla) → review+

Comment 11

7 years ago
You might want to file a help bug for this so we remember to add it in help.
(Assignee)

Comment 12

7 years ago
(In reply to comment #11)
> You might want to file a help bug for this so we remember to add it in help.

Filed bug 635806.
(Assignee)

Updated

7 years ago
Attachment #513846 - Flags: ui-review?(neil)
So, there are actually three preferences in play here:
> accessibility.browsewithcaret
Off by default, this actually reflects the state of the feature.
> accessibility.browsewithcaret_shortcut.enabled
On by default; if this is off, pressing F7 does nothing.
> accessibility.warn_on_browsewithcaret
On by default; if this is on, pressing F7 warns before turning the main preference on (it never warns before turning it off). If you tick the checkbox then this preference gets reset.

So I think we should go for two or maybe all three of these preferences:
Caret browsing enables you to navigate and select within pages using the cursor keys to move a visible caret.
[X] Use caret browsing
[X] Use the F7 shortcut to toggle caret browsing
    [X] Warn me before turning on caret browsing
(strings subject to bikeshedding)
Comment on attachment 513846 [details] [diff] [review]
Add checkbox to KeyNavigation for accessibility.browsewithcaret (F7-by-default mode) (v2)

>+                class="indent"
Don't see why this needs to be indented.

>+<!ENTITY browseWithCaretDesc.label "Enable keyboard navigation and selection within web pages using a visible caret.  The F7 key toggles this feature on/off.">
This shouldn't use "on/off", it should say "on or off".

>+<!ENTITY browseWithCaret.label "Always use the cursor keys to navigate within pages">
>+<!ENTITY browseWithCaret.accesskey "A">
"Always" seems to be a bit excessive to me, given that it's only until you press F7.
(Assignee)

Comment 15

7 years ago
Created attachment 514436 [details] [diff] [review]
Added accessibility.browsewithcaret group to keyboard Navigation preference panel.
Attachment #513846 - Attachment is obsolete: true
Attachment #514436 - Flags: ui-review?(neil)
Attachment #513846 - Flags: ui-review?(neil)
Comment on attachment 514436 [details] [diff] [review]
Added accessibility.browsewithcaret group to keyboard Navigation preference panel.

>+function UpdateBrowseWithCaretItems()
>+{
>+  document.getElementById("browseWithCaretWarn").disabled =
>+    !document.getElementById("accessibility.browsewithcaret").value ||
>+    document.getElementById("accessibility.browsewithcaret").locked;
>+
>+  document.getElementById("browseWithCaretShortCut").disabled =
>+    !document.getElementById("accessibility.browsewithcaret").value ||
>+    document.getElementById("accessibility.browsewithcaret").locked;
>+}
This isn't quite right; the Warn pref should be disabled if the ShortCut pref is false or if the Warn pref is locked; the ShortCut pref shouldn't need any special disabling.

>+      <preference id="accessibility.browsewithcaretshortcut"
>+                  name="accessibility.browsewithcaret_shortcut.enabled"
>+                  type="bool"
>+                  onchange="UpdateBrowseWithCaretItems();"/>
This is the only change that we're interested in.

>+    <groupbox>
I compared this to the existing groupbox which I noticed had align="start", which makes the checkboxes look marginally nicer...

>+      <vbox>
...but you also need to remove this unnecessary vbox.
Attachment #514436 - Flags: ui-review?(neil) → ui-review-
Comment on attachment 514436 [details] [diff] [review]
Added accessibility.browsewithcaret group to keyboard Navigation preference panel.

>+      <preference id="accessibility.browsewithcaretshortcut"
>+                  name="accessibility.browsewithcaret_shortcut.enabled"
Oops.
(Assignee)

Comment 18

7 years ago
Created attachment 514447 [details] [diff] [review]
Added accessibility.browsewithcaret group to keyboard Navigation preference panel. (v2)
Attachment #514436 - Attachment is obsolete: true
Attachment #514447 - Flags: ui-review?(neil)
(Assignee)

Updated

7 years ago
Attachment #514447 - Flags: ui-review?(neil)
(Assignee)

Comment 19

7 years ago
Created attachment 514466 [details] [diff] [review]
Added accessibility.browsewithcaret group to Keyboard Navigation preference panel (v3)
Attachment #514447 - Attachment is obsolete: true
Attachment #514466 - Flags: ui-review?(neil)
Comment on attachment 514466 [details] [diff] [review]
Added accessibility.browsewithcaret group to Keyboard Navigation preference panel (v3)

>+      <preference id="accessibility.browsewithcaret_shortcut"
>+                  name="accessibility.browsewithcaret_shortcut"
In case it wasn't clear, the correct name is accessibility.browsewithcaret_shortcut.enabled

>+      <checkbox id="browseWithCaretShortCut"
>+                class="indent"
This one doesn't need an indent. (I forgot to mention that last time, sorry.)

ui-r=me with those fixed.
Attachment #514466 - Flags: ui-review?(neil) → ui-review+
(Assignee)

Comment 21

7 years ago
Created attachment 514689 [details] [diff] [review]
Added accessibility.browsewithcaret group to Keyboard Navigation preference panel (v4)
Attachment #514466 - Attachment is obsolete: true
Attachment #514689 - Flags: ui-review+
Attachment #514689 - Flags: review?(iann_bugzilla)

Comment 22

7 years ago
Comment on attachment 514689 [details] [diff] [review]
Added accessibility.browsewithcaret group to Keyboard Navigation preference panel (v4)

>+++ b/suite/locales/en-US/chrome/common/pref/pref-keynav.dtd
>+<!ENTITY accessibilityBrowseWithCaret.label "Browse With Caret (F7-by-default mode)">
(F7-by-default mode) seems to have reappeared in this label somehow...

>+<!ENTITY browseWithCaretDesc.label "Caret browsing enables you to navigate and select within pages using the cursor keys to move a visible caret.">
>+<!ENTITY browseWithCaretUse.label "Use caret browsing">
>+<!ENTITY browseWithCaretUse.accesskey "c">
"U" would be a better key to use.
r=me with those addressed.
Attachment #514689 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 23

7 years ago
Created attachment 515063 [details] [diff] [review]
Added accessibility.browsewithcaret group to Keyboard Navigation preference panel (v4) [Checkin: comment 26]
Attachment #514689 - Attachment is obsolete: true
Attachment #515063 - Flags: ui-review+
Attachment #515063 - Flags: review+

Comment 24

7 years ago
ewong, is this ready to land? The mac keynav pane is empty after the removal of fayt prefs and it would be nice to see something in the pane :-)
(Assignee)

Comment 25

7 years ago
(In reply to comment #24)
> ewong, is this ready to land? The mac keynav pane is empty after the removal of
> fayt prefs and it would be nice to see something in the pane :-)

I believe so.   I've set the checkin-needed.
Keywords: checkin-needed
(Reporter)

Comment 26

7 years ago
Comment on attachment 515063 [details] [diff] [review]
Added accessibility.browsewithcaret group to Keyboard Navigation preference panel (v4) [Checkin: comment 26]

http://hg.mozilla.org/comm-central/rev/7df41cd35756

>+function UpdateBrowseWithCaretItems()
>+{
>+  document.getElementById("browseWithCaretWarn").disabled =
>+    !document.getElementById("accessibility.browsewithcaret_shortcut.enabled").value ||
>+    document.getElementById("accessibility.browsewithcaret").locked;
>+
>+}

I took the liberty to remove that blank line upon checkin.
Attachment #515063 - Attachment description: Added accessibility.browsewithcaret group to Keyboard Navigation preference panel (v4) → Added accessibility.browsewithcaret group to Keyboard Navigation preference panel (v4) [Checkin: comment 26]
(Reporter)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b3

Updated

7 years ago
Duplicate of this bug: 634645
You need to log in before you can comment on or make changes to this bug.