121704 - Caret continues to flash after element is no longer visible

Status: VERIFIED FIXED

(Core :: Web Painting, defect)

Description

[Sebastiano Vigna]

1) Go to the page above.
2) Move the mouse on the "Search" link on the right.
3) A new box with a text input element and a "Search" button appears.
4) Click in the text input element so that the cursor blinks.
5) Move away the mouse from the box; after 1/10s the box is gone, but the cursor
is still there, blinking; you can even write and see it move (but no characters
appear).

Comment 1

[TucsonTester3]

Happening in Build: 2002020103   OS: Win2k

Comment 2

[rods (gone)]

-->

Comment 3

[TucsonTester3]

Attachment: minimized testcase

Comment 4

[Kevin McCluskey (gone)]

Reproduced problem using 2002022003 build on WinXP.

Comment 5

[michael wendell]

Adding testcase keyword.

Comment 6

[Greg K.]

Reproduced using FizzillaMach/2003022003. Setting All/All.

A similar problem happens when switching between tabs (see bug 188452 and
probably others). Perhaps the cause is the same.

Comment 7

[Boris Zbarsky [:bzbarsky]]

.

Comment 8

[Boris Zbarsky [:bzbarsky]]

The site sets visibility, not display, which explains why the <input> continues
to have focus, if nothing else....  Should losing visibility cause things to
lose focus?

Comment 9

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

Why shouldn't hidden elements be able to have focus if the Web author wants it
that way?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Well, in that case we need to fix the "focused textfield, so I blink on top of
everything" thing... ;)

Comment 11

[Hixie (not reading bugmail)]

Yeah, the caret should be painted in the same layer as text for the element,
ideally...

Comment 12

[Boris Zbarsky [:bzbarsky]]

Well, the problem is that the caret _blinks_.  If I recall correctly, we
implement this by directly spewing stuff to the rendering context instead of
invalidating the whole text input (probably for perf reasons and to ensure a
constant blink rate).  See nsCaret::GetCaretRectAndInvert and the call path to
that from nsCaret::CaretBlinkCallback

Comment 13

[Hixie (not reading bugmail)]

"But that means our carret won't perfectly blend in with rgba() and opacity
styles in Web pages!"

Comment 14

[Boris Zbarsky [:bzbarsky]]

Damn right.  It doesn't, in fact -- an <input> with opacity 0.5 has a solid
black caret.

Comment 15

[Hixie (not reading bugmail)]

Yeah, so does one with opacity:0 -- that would be this bug, for all intents and
purposes. :-P

Comment 16

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

I've always hated the separate caret rendering path. It creates a worse bug than
this one: when a text input is logically topmost but contained in a native
widget that does not happen to be the topmost native widget, the caret can
disappear behind other native widgets. I've long been in favour of rewriting the
caret code to use the standard rendering path, but the owner (sfraser, I think)
did not agree.

Comment 17

[Boris Zbarsky [:bzbarsky]]

OK, what are the drawbacks of doing it through the normal rendering path?  Perf?

Comment 18

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

Yes, performance. To which I say that if we can't keep the caret blinking, then
we probably can't keep up with a fast typist so we already have a worse problem.

Comment 19

[Boris Zbarsky [:bzbarsky]]

roc, we _can't_ keep up with a fast typist, last I checked. We have bugs on
that, though... ;)

If we were to use "normal" drawing, how would we do that?  Get the closest view
containing our caret and call UpdateView() on it with the caret rect?  That's
what <blink> does...

It may be worth trying this (maybe in 1.7a?) and seeing whether the perf issues
materialize....

Comment 20

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

I think that text frames, just as they paint the selection, should also paint
the caret (which is really just a collapsed selection) during their Paint()
method. When you move the caret or blink the caret, you invalidate its text frame.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Hm... Does the caret always have a text frame attached?  Say I'm between the "e"
and the "f" in the following document I have open in Editor:

<span>abcde</span><span>fghi</span>

What does the selection setup look like?

But yes, that sounds like a reasonable approach...

Comment 22

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

I don't really know, and I think there may be hacks where the caret is
positioned outside of any text frame. But that implies that caret behavior is
poorly defined and/or buggy when the text frames have views that set 'opacity'
or whatever.

Comment 23

[Boris Zbarsky [:bzbarsky]]

Heh.  True....

So do we have a sacrificial lamb willing to change how caret works?  ;)

Comment 24

[Simon Fraser [no longer active]]

The caret is often drawn outside of frames, and needs to be shown on lines with
a single zero-width <br> frame, etc. I think you'll have a lot of trouble trying
to draw the caret via the frame drawing codepath.

There are also usability issues if you go this route. Imaging typing in a
composer with floats, when you typing target is underneath a float. Do you want
th caret to be visible in that case? We thought so.

Note also that when double buffering is in effect, the caret is not drawn to the
backbuffer, but directly to the screen (again for performance).

The bug shown in the testcase should be pretty easy to fix.

Comment 25

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

> The caret is often drawn outside of frames, and needs to be shown on lines
> with a single zero-width <br> frame, etc. I think you'll have a lot of trouble
> trying to draw the caret via the frame drawing codepath.

Indeed we may...

> Imaging typing in a composer with floats, when you typing target is
> underneath a float. Do you want th caret to be visible in that case? We
> thought so.

Yes, I guess that's marginally better than hiding it. On the other hand, in this
bug we want the caret to be hidden when it's behind something. I guess we want
the caret to be the topmost thing in its editor but underneath whatever's on top
of the editor. Which gives me an idea...

> The bug shown in the testcase should be pretty easy to fix.

Yes but the bug where a topmost text input's caret gets lost under a native
widget is not easy to fix. I wish I could find the bug number but I've lost it...

So I think the thing to do is to paint the caret in the editor's frame, after
all the normal children have been painted. I'm not familiar with the structure
of frames produced by editors, but there should be a place we can hook in. This
should be fairly easy to implement and deal with all of Simon's concerns, except
maybe performance, but I'm not convinced we should worry about that one.

Comment 26

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

This implies that if you're editing a document containing a translucent element
and the caret is in the element, then the caret will not be translucent.
However, if you're editing a document and the whole editor is translucent, then
the caret will also be translucent. That sounds right to me.

Comment 27

[Simon Fraser [no longer active]]

> but I'm not convinced we should worry about {performance} 

See bug 194343. Caret performance is an important issue, because it affects
idle-time CPU usage.

I'm not sure what you mean by the 'editor frame'. For composer, would this be
the root frame? Note that you can also make the caret display in browser
windows, so you should't make caret drawing specific to editing.

Comment 28

[Boris Zbarsky [:bzbarsky]]

There is no special "Editor frame" in text inputs.  The editor is just hooked up
to the anonymous div inside the <input>.

Comment 29

[Uri Bernstein]

Fixed by bug 287813
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060418 Firefox/3.0a1

Comment 30

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using build 2006-04-28-05 of SeaMonkey trunk under Windows XP.