After clicking on readonly text input, caret doesn't blink anymore in caret mode

RESOLVED FIXED in mozilla24

Status

()

RESOLVED FIXED
11 years ago
4 years ago

People

(Reporter: martijn.martijn, Assigned: masayuki)

Tracking

({regression, testcase})

Trunk
mozilla24
x86
Windows XP
regression, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

11 years ago
Created attachment 281296 [details]
testcase

See testcase, to reproduce the bug:
- Turn on caret browsing (F7)
- Click on the readonly text input
- Click on the text in this page

Expected result:
Caret should blink

Actual result:
Caret does not blink (readonly)

This regressed between 2005-10-03 and 2005-10-04, a regression from bug 193316.
I guess some similar code is needed in nsTextEditorFocusListener::Blur as was added in nsTextEditorFocusListener::Focus.
Created attachment 752096 [details] [diff] [review]
Patch

The actual cause is that nsEditor::FinalizeSelection() doesn't call nsISelectionController::SetCaretReadOnly(false). The instance of selection controller of <input> or <textarea> is different from the instance of presShell. However, some methods delegate the handling to presShell. Therefore, like this, change for the selection controller of <input> or <textarea> causes the caret behavior of caret browsing.

Similarly, current implementation doesn't reset the nsCaret::SetCaretDOMSelection().

By these reasons, I think that nsFocusManager should have a method which reset the caret and selection for caret browsing mode. And when an editor loses focus, the editor should call it.

Ideally, this patch should be reviewed by enn too, but he isn't available until June 10...
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Attachment #752096 - Flags: review?(ehsan)

Comment 2

6 years ago
Comment on attachment 752096 [details] [diff] [review]
Patch

Review of attachment 752096 [details] [diff] [review]:
-----------------------------------------------------------------

Can you please add a test for this as well?
Attachment #752096 - Flags: review?(ehsan) → review+
Do you have idea for testing if the caret is blinking?
Flags: needinfo?(ehsan)

Comment 4

6 years ago
Yes, use http://mxr.mozilla.org/mozilla-central/source/layout/base/tests/test_reftests_with_caret.html?force=1.  That test disables the blinking and renders the caret at all times, to avoid timing issues introduced because of the blinking.
Flags: needinfo?(ehsan)

Comment 5

6 years ago
(Note that if your test doesn't require chrome privileges, you can just create a simple reftest.  Reftests have the ui.caretBlinkTime set to -1 as well.)
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #4)
> Yes, use
> http://mxr.mozilla.org/mozilla-central/source/layout/base/tests/
> test_reftests_with_caret.html?force=1.  That test disables the blinking and
> renders the caret at all times, to avoid timing issues introduced because of
> the blinking.

The bug of this is what the caret is NOT blinking after read-only editor loses focus. I.e., we need to check the caret is actually *blinking* at that time. It seems that setting ui.caretBlinkTime to -1 stops blinking the caret. Therefore, I don't think it can test this bug, isn't it?
Flags: needinfo?(ehsan)

Comment 7

6 years ago
(In reply to comment #6)
> (In reply to :Ehsan Akhgari (needinfo? me!) from comment #4)
> > Yes, use
> > http://mxr.mozilla.org/mozilla-central/source/layout/base/tests/
> > test_reftests_with_caret.html?force=1.  That test disables the blinking and
> > renders the caret at all times, to avoid timing issues introduced because of
> > the blinking.
> 
> The bug of this is what the caret is NOT blinking after read-only editor loses
> focus. I.e., we need to check the caret is actually *blinking* at that time. It
> seems that setting ui.caretBlinkTime to -1 stops blinking the caret. Therefore,
> I don't think it can test this bug, isn't it?

Oh, you're right.  Nevermind then, feel free to land this without a test.  :/
Flags: needinfo?(ehsan)
https://hg.mozilla.org/mozilla-central/rev/76d9db6490f4
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
This bug seems to be back: the testcase in comment 0 is broken again.
(In reply to Pierre de LA MORINERIE from comment #10)
> This bug seems to be back: the testcase in comment 0 is broken again.

Please file a new bug. It's really different bug.

Comment 12

4 years ago
I think the bug need to be reopened.
The cursor still appears although the input text field is read only.
(Reporter)

Comment 13

4 years ago
(In reply to Bassel from comment #12)
> I think the bug need to be reopened.
> The cursor still appears although the input text field is read only.

What do you mean? In the text input, the caret should appear, but it should not blink.
You need to log in before you can comment on or make changes to this bug.