1373788 - The caret stops blinking in the entire tab as soon as a text field is unfocused

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Markus Stange [:mstange]]

Steps to reproduce:
 1. Go to https://bugzilla.mozilla.org/enter_bug.cgi?product=Core&component=Gecko%20Profiler
 2. Please the cursor into the description text box and observe that the caret blinks.
 3. Click into the "Blocks:" text field a few rows below.

Now the caret stops blinking. Changing focus to a different text field doesn't restore the blinking behavior.
Blinking still works in all other tabs, even ones that run in the same content process as this tab.

I don't know whether this is a regression.

Comment 1

[Markus Stange [:mstange]]

There's nothing special about enter_bug.cgi. This happens as soon as you move focus out of a text field.
For example it's reproducible on data:text/html,<input><input>

Comment 2

[Alice0775 White]

[Tracking Requested - why for this release]: caret blinking is broken

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=dfee29c06556ebe2f4ac43cf0744be9e93390d1d&tochange=db8a48dae370605e2b6de21326f882ef839401fd

Regressed by: db8a48dae370	Ehsan Akhgari — Bug 1372761 - Avoid needlessly grabbing the timer thread lock when selections change in order to reset the caret timer; r=mats

Comment 3

[(no longer active)]

Attachment: Reset the cached blink rate when the caret stops blinking

Otherwise the next time in ResetBlinking() we mistakenly think
we already have the blinking timer set.

Comment 4

[Mats Palmgren (inactive)]

Could you also make a regression test for this if possible?
layout/base/tests/test_reftests_with_caret.html is probably a good place.

Comment 5

[(no longer active)]

Sure, but I'll land the patch now as it's a pretty bad regression.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c90a327a4da0
Reset the cached blink rate when the caret stops blinking; r=mats

Comment 7

[(no longer active)]

(In reply to Mats Palmgren (:mats) from comment #4)
> Could you also make a regression test for this if possible?
> layout/base/tests/test_reftests_with_caret.html is probably a good place.

Actually I can't think of a good way to test this, since we disable caret blinking here: <https://searchfox.org/mozilla-central/rev/b95a1a95838cc6e85ea5d9b9e3ae39b4cd69d8ef/layout/base/tests/test_reftests_with_caret.html#334>

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/c90a327a4da0

Comment 9

[Marcia Knous [:marcia]]

Tracking 56+ per Comment 2.

Comment 10

[(no longer active)]

(In reply to :Ehsan Akhgari (needinfo please, extremely long backlog) from comment #7)
> (In reply to Mats Palmgren (:mats) from comment #4)
> > Could you also make a regression test for this if possible?
> > layout/base/tests/test_reftests_with_caret.html is probably a good place.
> 
> Actually I can't think of a good way to test this, since we disable caret
> blinking here:
> <https://searchfox.org/mozilla-central/rev/
> b95a1a95838cc6e85ea5d9b9e3ae39b4cd69d8ef/layout/base/tests/
> test_reftests_with_caret.html#334>

Moving the needinfo.

Comment 11

[(no longer active)]

And closing the bug because now I'm getting "release tracking alert" emails for it...

Comment 12

[Mats Palmgren (inactive)]

Yeah, I can't think of a way to test this either.  I don't think it's possible
to make the reftest screenshot hit the caret off-cycle reliably.  I suspect
that even a very long "ui.caretBlinkCount" would cause a lot of intermittent
failures.

A bug like this is will be detected by humans fast enough anyway. :-)

Comment 13

[(no longer active)]

Yeah...  Our caret testing already depends quite a bit on human testing anyway, so I'm not too sad about this one additional case.