261054 - Cursor size too wide in a <textarea>

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Frank]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20040913 Firefox/0.10
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20040913 Firefox/0.10

In a <textarea> the cursor is much wider then in <input type="text"> item of a form.

This makes it hard to determine the right position of the cursor with small
chars or a dot, when a proportinal font is being used.

Reproducible: Always
Steps to Reproduce:
1. Go to a page with <input type="text"> and <textarea> items in a form.
2. Type text in both boxes en notice the difference in the cursor size
3. The problem is bigger when a proportational font is being used.




Expected Results:  
The cursor size of the <input> should also be used in the <textarea>

Comment 1

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.3) Gecko/20040922
Firefox/0.10

Confirming ->NEW 

The cursor in <textarea> is 2 pixels wide vs 1 pixel in other elements

Comment 2

[Peter van der Woude [:Peter6]]

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

Comment 3

[Aaron Leventhal]

It's also 2px wide in composer and Nvu. I believe these varying caret widths are
what's messing up the Window-Eyes screen reader's caret detection.

Daniel, do you mind if we make the caret always 1 px wide?

Comment 4

[Aaron Leventhal]

Aha! Believe it or not, we purposely make multi line textarea carets 2px and
single line textfield carets 1px. This was implemented in bug 39529.

However, the biggest reason I can see for it is that there are some other MS
apps which have varying caret widths, although that's not consistent. For
example, wordpad and notepad always have a 1px wide caret.

One reason might be that in a large text area it's hard to find a thin 1px
caret, so a thicker more visible one is used.

(Snipped from bug description)
> This makes it hard to determine the right position of the cursor with small
> chars or a dot, when a proportinal font is being used.
Reporter: can you attach a testcase where the 2px wide caret creates this problem?

Comment 5

[Aaron Leventhal]

Let's change this. The caret should always be 1px wide. That's what IE does, and
it'll fix the screen reader tracking problem. There's no real good reason for
having it be 2px sometimes.

Comment 6

[Aaron Leventhal]

Attachment: Make caret always 1 px

Comment 7

[Aaron Leventhal]

Comment on attachment 161094 [details] [diff] [review]
Make caret always 1 px

Should I get rid of the look and feel code since it's always 1 now? It's just
extra bloat imo. Or just leave it there in case, some day, some platform wants
2px?

Comment 8

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

aaronlev: the fix could be a problem for Nvu/Composer is the document/element
has a background that makes a thin caret almost invisible, so let me try it in
Nvu please.
I am also concerned for very high res screens.
I don't really understand why you want to change LookAndFeel and remove
SetCaretWidth for instance. One embedder may want to have its own caret size for
a reason we cannot imagine right now.
Last point, what's the impact on bidi ? On my latin1 windows box, the cursor
changes in a rtl document: there's a 2x2 pixels thingy attached to the top of
the cursor to indicate the writing's direction. Shouldn't that become 1x1 too ?
In that case, isn't 1x1 too small and almost invisible ?

Comment 9

[Aaron Leventhal]

(In reply to comment #8)
> aaronlev: the fix could be a problem for Nvu/Composer is the document/element
> has a background that makes a thin caret almost invisible, so let me try it in
> Nvu please.
Didn't think of that.

> I am also concerned for very high res screens.
Doesn't seem to be a problem for single line text fields. We're just making it
consistent.

> I don't really understand why you want to change LookAndFeel and remove
> SetCaretWidth for instance. One embedder may want to have its own caret size for
> a reason we cannot imagine right now.
Okay. Just so many what if's that never get used sometimes. As John Morrison
told me once, it's not the first cookie that makes you fat :) Doesn't matter to
me much either way.

> Last point, what's the impact on bidi ? On my latin1 windows box, the cursor
> changes in a rtl document: there's a 2x2 pixels thingy attached to the top of
> the cursor to indicate the writing's direction. Shouldn't that become 1x1 too ?
Didn't think of that. Will you take a look when you try out the patch? For your
test build you might want to keep the old guids in nsISelectionController and
nsICaret to shorten the build process. Up to you.

> In that case, isn't 1x1 too small and almost invisible ?
It would be, but wouldn't that also be a problem in single line text fields now?

Comment 10

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

aaronlev: with your patch, the direction control shown in the caret becomes
automatically 1x1 (I guess it's drawn using the caret's size) and it's
absolutely invisible...
I have also checked that most text processors use a 2 pixels caret while text
editors use a 1 pixel caret. So that's a "go ahead" if you want textareas to
have a  1 pixel caret but in the case of Composer, Nvu and more generally the
HTML editor, it should remain 2px.

Comment 11

[Aaron Leventhal]

(In reply to comment #10)
> aaronlev: with your patch, the direction control shown in the caret becomes
> automatically 1x1 (I guess it's drawn using the caret's size) and it's
> absolutely invisible...
But what happens in a 1 line textfield now with the direction control? Isn't
that a problem?

> I have also checked that most text processors use a 2 pixels caret while text
> editors use a 1 pixel caret. 
Wordpad and notepad use 1px in the content area.

> So that's a "go ahead" if you want textareas to
> have a  1 pixel caret but in the case of Composer, Nvu and more generally the
> HTML editor, it should remain 2px.
The lack of consistency is still going to cause a problem with the Windows
screen readers who expect a consistent pixel width throughout a given window
class. We'll also have to decide what contenteditable gets.

If the direction control is a problem in one place with 1px, isn't it a problem
in all of the places? Or does the direction control only occur in rich text areas?

It sounds like becaues of the direction control issue we might need to consider
the idea of using 2px width for all of our carets. We need consistency or we
need the screen reader vendors to fix their stuff. Or, we need to have a
different window class depending on whether the width is 1px or 2px, but that
will break mouse wheel scrolling.

Comment 12

[Aaron Leventhal]

Attachment: Make caret 1px (still obeys look and feel), but make sure bidi direction indicator is at least 2px

Hope this looks okay.

Comment 13

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 161213 [details] [diff] [review]
Make caret 1px (still obeys look and feel), but make sure bidi direction indicator is at least 2px

Seems ok to me.
r=[e-mail]

I am still uncertain about the remove of setCaretWidth() though.

Comment 14

[[e-mail]]

Comment on attachment 161213 [details] [diff] [review]
Make caret 1px (still obeys look and feel), but make sure bidi direction indicator is at least 2px

Looks fine to me but aren't you supposed to get layout review on the layout
bits?

>+          if (! (flags & nsIPlaintextEditor::eEditorReadonlyMask)) {
>             selCon->SetCaretEnabled(PR_TRUE);
>           }
/me repeats his grumble about lack of focus in readonly textfields

Comment 15

[Aaron Leventhal]

Comment on attachment 161213 [details] [diff] [review]
Make caret 1px (still obeys look and feel), but make sure bidi direction indicator is at least 2px

Neil, who do you suggest I get sr= for the layout bits from.

Selection is really a separate piece from layout, and all of our selection code
jockeys are gone now.

I could ask bryner.

Comment 16

[[e-mail]]

rbs might be a good superreviewer, although as per bug 262639 comment 4 you
should rename the pref to ui.caretWidth

Comment 17

[Aaron Leventhal]

I can rename it from ui.caretWidthPixels to ui.caretWidth if you think that's
better.

Comment 18

[rbs]

Comment on attachment 161213 [details] [diff] [review]
Make caret 1px (still obeys look and feel), but make sure bidi direction indicator is at least 2px

sr=rbs, with the renaming, in which case you can simply mark the other bug as
dup.

I don't have a strong opinion one way or another for |setCaretWidth| too. At
least the pref provides a crude workaround. It could be reinstanted if there is
a demand (which I doubt).

Comment 19

[Aaron Leventhal]

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

Comment 20

[Aaron Leventhal]

2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ editor/ libeditor/ text/
nsEditorEventListeners.cpp 	1.231 	1/17  	Bug 261054. Make caret consistently
the same width throughout Gecko/Mozilla. Configurable through ui.caretWidth.
r=glazou, sr=rbs
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ extensions/ typeaheadfind/
src/ nsTypeAheadFind.cpp 	1.100 	0/4 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ extensions/ xmlterm/ base/
mozXMLTerminal.cpp 	1.80 	0/11 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ layout/ base/ public/
nsICaret.h 	1.33 	1/6 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ widget/ src/ gtk/
nsLookAndFeel.cpp 	1.62 	1/2 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ layout/ base/ src/
nsCaret.cpp 	1.133 	13/26 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ layout/ base/ src/
nsCaret.h 	1.39 	3/3 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ widget/ src/ mac/
nsLookAndFeel.cpp 	1.57 	1/2 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ widget/ src/ windows/
nsLookAndFeel.cpp 	1.47 	1/4 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ widget/ src/ os2/
nsLookAndFeel.cpp 	1.35 	1/4 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ widget/ src/ xlib/
nsLookAndFeel.cpp 	1.28 	1/2 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ widget/ src/ xpwidgets/
nsXPLookAndFeel.cpp 	1.41 	1/1 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ widget/ src/ gtk2/
nsLookAndFeel.cpp 	1.18 	1/2 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ widget/ public/
nsILookAndFeel.h 	1.43 	4/5 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ widget/ public/
nsWidgetsCID.h 	3.43 	3/3 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ layout/ html/ forms/ src/
nsTextControlFrame.cpp 	3.178 	0/18 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ widget/ src/ beos/
nsLookAndFeel.cpp 	1.26 	1/4 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ widget/ src/ photon/
nsLookAndFeel.cpp 	1.31 	2/3 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ layout/ html/ base/ src/
nsPresShell.cpp 	3.781 	0/8 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ content/ base/ public/
nsISelectionController.idl 	3.21 	1/8 
2004-10-12 18:12	aaronleventhal%moonset.net 	mozilla/ content/ events/ src/
nsEventStateManager.cpp 	1.537 	0/8 

Comment 21

[Frank]

Correct my if I'm wrong, but is this only fixed in the trunk?

I've downloaded (20-Oct-2004) the nightly build from /latest-trunk and
/latest-0.9. This bug is fixed in the trunk (which announces as 1.0PR+ during
installation) and not in the 0.9 (announces as 1.0 RC1)

Comment 22

[Frank]

This bug is fixed in the trunk, but NOT in the 1.0 branch. Seems not too
difficult to fix it in the branch as well.

Comment 23

[Aaron Leventhal]

I don't want to risk this on the branch.

Comment 24

[Aaron Leventhal]

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