49508 - Can't make caret visible in browser

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect, P3)

Description

[Aaron Leventhal]

I need to be able to view the caret in the browser for debugging purposes.
The project I need this for is Mozilla for blind users. I always need to know 
where the collapsed selection is. I also need to make sure it moves with regular 
arrow key presses.

Comment 1

[Gervase Markham [:gerv]]

Hey, Aaron :-) How come you don't have proper Bugzilla privs? Anyway, fixed.

Confirming for evaluation.

Gerv

Comment 2

[Aaron Leventhal]

Simon,
There seems to be a related problem.
I've also found it impossible to extend the selection in the browser using:
/* This code works in the editor */
var controller = 
 document.commandDispatcher.getControllerForCommand('cmd_selectCharPrevious');
controller.doCommand('cmd_selectCharPrevious');
Do you think this is the same bug?

Comment 3

[Simon Fraser [no longer active]]

No, different bug. cmd_selectCharPrevious will only work in editors (text fields, 
composer).

Comment 4

[Akkana Peck]

Is there any way to tell the browser that you want to register a selection
controller (not an editor controller) on it?  Keyboard-only users will need
commands like selectCharPrevious in order to extend the selection via the
keyboard.

Comment 5

[Simon Fraser [no longer active]]

Ooops, so I lied. There should be a controller for the browser window that 
handles cmd_selectCharPrevious and friends. See
http://lxr.mozilla.org/seamonkey/source/dom/src/base/nsGlobalWindow.cpp#4731

Comment 6

[anthonyd]

should this be RTM? It seems like a user accessability issue, is this important 
enough for rtm?

anthonyd

Comment 7

[anthonyd]

setting to future, with the understanding simon can knock it to m19 if he feels 
this is important to the rtm release.

anthonyd

Comment 8

[Simon Fraser [no longer active]]

Mozilla0.9.

Comment 9

[Matthew T (active 1999-2002)]

Actually, this shouldn't be just for debugging purposes, it should be a visible 
option -- otherwise users who can't use a pointing device will be practically 
unable to begin a selection, since they can't see where the caret is.

With this option turned on, the arrow keys should move the caret rather than 
scrolling the viewport.

Blocks compliance with UAAGs (bug 24413), checkpoint 1.3.

Comment 10

[Aaron Leventhal]

Actually, I have managed to fix this myself with a few hints from Akkana.
(Thanks akk).

The only problem now is I'd like to gray the caret out for readonly textfields
and for the browser. I wanted to use nsICaret::SetCaretReadOnly(PR_BOOL) to set
this state, which doesn't really work. I think the problem is that there is one
global system caret, so as soon as the focus goes to the URL bar, the readonly
setting changes to false. Therefore the readonly state would need to be tracked
in more places, or by the focus manager. Do I have this right?

Comment 11

[Simon Fraser [no longer active]]

You need to set and unset the caret's readonly state just as the caret width is 
set and unset; indeed, perhaps you could generalize the call to SetCaretWidth to 
SetCaretProperties or somesuch.

Comment 12

[Simon Fraser [no longer active]]

Over to Aaron, who has a grasp on this.

Comment 13

[Aaron Leventhal]

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

Comment 14

[Aaron Leventhal]

I'm almost done with this fix.

This allow users of screen readers and other accessibility
tools to browse Mozilla as if it's a read-only editor. 

Rather than arrow keys scrolling the page up, down, left and right, they move a
visible caret. Shifted movement keys highlight a selection. This way selection
can be done without the mouse. This involves changes to nsGlobalWindow.cpp and
htmlBindings.xml.

The visible caret is set to "readonly" so that it's gray and doesn't blink. This
indicates to the user that no data entry is allowed. This involves changes in
nsCaret.cpp.

If the user presses tab or shift-tab, the focus is moved relative to the
position of the caret. In addition, whenever the focus moves, the caret is
placed at the start of the focus. When the user moves the caret over a link, the
link is focused. This involves changes in nsEventStateManager.cpp.

I'm 90% done with this coding. All that remains is to ensure the readonly state
is properly maintained when focus changes.

Comment 15

[Aaron Leventhal]

Attachment: First attempt, doesn't make any new caret methods like SetCaretProperties

Comment 16

[Aaron Leventhal]

Attachment: Finally! Fixed problem with caret not showing up when there's a text field in content

Comment 17

[timeless]

@@ -2944,6 +3080,40 @@
...
+        if (!aContent) {
...
+          }
^ inconsistent {} indentation relative to file
v Please avoid this. Just add an extra line at the end.
\ No newline at end of file
Index: layout/events/src/nsEventStateManager.h

+  else
   mRendContext->SetColor(NS_RGB(255,255,255));

RCS file: /cvsroot/mozilla/layout/xbl/builtin/htmlBindings.xml,v
is it me, or do you reget the pref object for each key? that sounds expensive.

note to self[not your bug] 
nsCAutoString(sCopyString).EqualsWithConversion(aCommand)
is yucky.

fwiw, would you mind if i created an access.js for accessibility prefs?

otherwise, your stuff looks cool :-)

Comment 18

[Aaron Leventhal]

Attachment: Performance improved: no JS needed in XBL for keybindings, prefs cached

Comment 19

[Simon Fraser [no longer active]]

nsEventStateManager.cpp:
I'd suggest factoring the code under the if (mBrowseWithCaret) tests, especially 
the key handling blocks, into new, protected methods. It leaves the rest of the 
code much more readable.

Possible leak here:

+          nsIDOMRange* newRange;
+          nsresult rv = rangeDoc->CreateRange(&newRange);
+          if (NS_SUCCEEDED(rv)) {
+            // If we could create a new range, then set it to the current focus 
node
+            // And then collapse the selection
+            newRange->SelectNodeContents(currentFocusNode);
+            domSelection->AddRange(newRange);
+            domSelection->CollapseToStart();
+          }

Fix this:
+}
\ No newline at end of file


htmlBindings.xml
+      <handler event="keypress" keycode="VK_LEFT" modifiers="control" command=
"cmd_wordPrevious" />
+      <handler event="keypress" keycode="VK_RIGHT" modifiers="control" command=
"cmd_wordNext" />
etc
Don't you need platform-specific bindings here? Mac's word-move is option-arrow.

Comment 20

[Matthew T (active 1999-2002)]

> Don't you need platform-specific bindings here? ...

Does this mean there will be *three* different places where keybindings for text 
navigation are stored -- one for read-only HTML (as created by this bug), one for 
Composer, and one for XP Toolkit? Surely that's just asking for unintentional 
inconsistency -- it already caused bug 66135, for example. Smells like some 
refactoring is needed.

Comment 21

[Aaron Leventhal]

Attachment: Factors out code into new methods; also moves keybindings into platform specific areas

Comment 22

[Simon Fraser [no longer active]]

In GlobalWindow's ctor, you need to init mBrowserWithCaret to PR_FALSE in case 
the getting of the prefs service fails. Otherwise, r=sfraser

Comment 23

[Chris Waterson]

Looks good: break up some of the dense code with a bit of vertical whitespace
and be sure to use brace style consistent with the rest of the file. Be sure not
to monkey with user-agent string in all.js. Maybe note/comment how to extend
CaretFollowsFocus() to handle new tags in the future. Do that stuff, and sr=waterson

Comment 24

[Blake Ross]

And please use direct initialization instead of copy (see 67860).

Comment 25

[Aaron Leventhal]

Attachment: Final version, after Waterson's & scc's comments

Comment 26

[Aaron Leventhal]

Fix checked in to tip.
To test, add line to prefs.js
user_pref("accessibility.browsewithcaret",true);