420987 - Textarea caret invisible at certain zoom levels on Windows

Status: VERIFIED FIXED

Description

[[PTO until 15 Jan] Chris Pearce (:cpearce)]

Created attachment 307364 [details]
Testcase - Caret invisible at zoom-out level 1 on Windows.

The textarea's caret is invisible at one zoom out with specific body margin and padding. This only happens on Windows, and only happens with specific margin and padding values on the body tag. I discovered this because it happens with bugzilla's comment textarea, i.e. on Windows at zoom-out level one the caret is not visible in bugzilla's comment field.

This bug exists at least as far back as FF3b3, only on Windows.

STR:
 
1. Load testcase.
2. Zoom out page by 1 (CTRL+MouseDown).
3. Focus textarea, note that no caret appears.
4. Type something, you'll see your typed text appear in the textarea, but still there's no caret.

Comment 1

[[PTO until 15 Jan] Chris Pearce (:cpearce)]

Requesting triage... Roc?

Comment 2

[David Baron :dbaron: ⌚️UTC-8]

I've seen missing caret when zoomed out on Linux.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Should be an easy fix, just change the caret drawing code a bit to manually span the caret rect to pixels before drawing directly to gfxContext, and refuse to make it disappear altogether.

Comment 4

[Henrik Skupin (:whimboo)]

Same happens with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008041004 Minefield/3.0pre ID:2008041004

The caret is invisible on OS X when the textarea doesn't contain any text. Further it happens only for each second decrease/increase.

Comment 5

[Aaron Leventhal]

Screen reader usage breaks for current generation Windows screen readers when this happens -- the user cannot read the text in the text area. Newer screen readers are moving to an engineered interface for the caret pos and don't have the problem.

Comment 6

[Aaron Leventhal]

Created attachment 315203 [details] [diff] [review]
Follow roc's suggestion from comment 3

This is too annoying to ship with. If the code needs to be changed a lot I would appreciate if someone else takes it from here.

Anyway, this basically does what Roc said in comment 3 and fixes the problem for me.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is the right idea but nsDisplayCaret needs to be changed so that its bounds reflect the increased width of the caret. I think the correct thing to do would be to change the width of mCaretRect itself so that it's a minimum of one device pixel wide.

Comment 8

[Aaron Leventhal]

I don't have time to twiddle with this patch, I'll let someone more familiar with the area do the rest from here.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Ah, we actually do set the caret width to be one device pixel right now. We just don't update it dynamically when zoom changes.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Created attachment 315445 [details] [diff] [review]
fix

I think this is the way to go. nsCaret needs to cache less.

All in all, I think a pretty safe patch.

Comment 11

[Blake Kaplan (:mrbkap)]

Comment on attachment 315445 [details] [diff] [review]
fix

Is it worth passing the metrics computed in UpdateCaretRects to UpdateHookRect? r+sr=me either way.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Created attachment 315460 [details] [diff] [review]
fix with comments

Yeah, might as well pass the Metrics down to UpdateHookRect.

This is a simple low-risk patch that apparently will fix an accessiblity issue if someone zooms out.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in.

Comment 14

[Henrik Skupin (:whimboo)]

Verified with as much as possible zoom levels. Works as expected with the following builds:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041506 Minefield/3.0pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008041504 Minefield/3.0pre ID:2008041504