1950748 - Windows Text Cursor Indicator does not disappear from text inputs when focus leaves

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[Nathan LaPré]

STR:

1. Turn on UIA support in about:config; set accessibility.uia.enable to true.
2. Turn on the Windows Text Cursor Indicator (Settings -> Accessibility -> Text Cursor -> Set Text cursor indicator to "On")
3. Open test page data:text/html,<input value="ABCDEF"/>
4. Using the mouse, click in the text box between the A and the B (to the right of the A but to the left of the B).
5. Click outside of the text box so that focus leaves the text input

Expected: Text cursor indicator disappears from the text input.
Actual: Text cursor indicator remains in the text input, and at an unexpected location.

This works in Edge; the text cursor indicator disappears. It somewhat works using the IA2 -> UIA bridge in Firefox, but it's broken in other, worse ways. The text cursor indicator should disappear, but also puzzling is that it remains in an unexpected and inconsistent location; sometimes at the very end, sometimes at the very start, sometimes somewhere in the middle. It seems to move to the start of the text for <input>s, and the end of the text for <div contenteditable>s. Weird. Regardless, this might be beside the point: I think the indicator should disappear, and it doesn't.

Comment 1

[James Teh [:Jamie]]

1. Perhaps Text Cursor Indicator only listens for TextSelectionChanged events, not Focus events. Perhaps we're not firing a TextSelectionChanged event when focus leaves a text box.
   • One challenge is that documents can have a caret, even if caret browsing is disabled. Presumably that caret is invisible. What does Text Cursor Indicator do in that case? This also applies to Edge.
2. Alternatively, perhaps we're still reporting that the caret is in a text box even after focus moves somewhere else.

Comment 2

[James Teh [:Jamie]]

I wonder if bug 1967379 fixed or improved this.

Comment 3

[James Teh [:Jamie]]

Morgan, would you be able to test this with latest Nightly? Thanks.

Comment 4

[James Teh [:Jamie]]

And if it doesn't work, does the Text Cursor Indicator ever disappear once it has landed in a text box? For example, what happens if you tab away from a text box rather than clicking? And does this only fail if you click in certain spots in a text box first, or does it fail always if you click outside of the text box regardless of what you did earlier?

Comment 5

[Morgan Reschenberg [:morgan]]

(In reply to James Teh [:Jamie] from comment #4)

And if it doesn't work, does the Text Cursor Indicator ever disappear once it has landed in a text box? For example, what happens if you tab away from a text box rather than clicking?

Tabbing away clears the cursor indicator; both when I move focus from content back to chrome and when I add another focusable element to content and tab to it after using my mouse to set the text cursor indicator inside the input box.

And does this only fail if you click in certain spots in a text box first, or does it fail always if you click outside of the text box regardless of what you did earlier?

In the example with only the text input, regardless of where I click first, once I click into the text input I can't clear the text cursor by clicking somewhere else. It persists if I: open a new tab with my mouse, switch apps and then switch back to fx, click another text input like the URL bar and then hit ESC twice to collapse the URL bar autocomplete and remove focus from the URL bar.

FWIW if there is another element in content (like <button>hello or just hello), this problem doesn't occur. If I first place the text cursor indicator in the input and then click the blank are on the web page, the cursor disappears.

Comment 6

[James Teh [:Jamie]]

(In reply to Morgan Reschenberg [:morgan] from comment #5)

FWIW if there is another element in content (like <button>hello or just hello), this problem doesn't occur.

Is this only the case if the other element is after the input? That is, if you put it before the input:

data:text/html,<button>hello</button><input value="abc">

I'm guessing you get the same bug if you click in the blank area of the document somewhere after the input?

Comment 7

[Morgan Reschenberg [:morgan]]

(In reply to James Teh [:Jamie] from comment #6)

(In reply to Morgan Reschenberg [:morgan] from comment #5)

FWIW if there is another element in content (like <button>hello or just hello), this problem doesn't occur.

Is this only the case if the other element is after the input? That is, if you put it before the input:

data:text/html,<button>hello</button><input value="abc">

I'm guessing you get the same bug if you click in the blank area of the document somewhere after the input?

If I use the example you gave above, placing the cursor between "a" and "b" and then clicking out of the input sends the cursor to the space between the "i" of the button and the "a" of the input

Comment 8

[James Teh [:Jamie]]

When you click outside a text input and there's no other content, Edge's caret is still inside the input just like Firefox's is, since there's nowhere else for it to go. Edge doesn't fire a caret event when you do this. However, Firefox does. We probably shouldn't fire that event in this case, since it isn't really correct.

Comment 9

[James Teh [:Jamie]]

1. The caret event we receive is actually for (DocAccessible, 1); i.e. the end of the document. The problem is that TextLeafPoint can't handle this.
2. ToTextLeafPoint calls GetChildAtOffset, which returns the text input, even though it's actually after the text input. Because it's a HyperText, it then descends to the first character therein. At the very least, it should descend to the last character. But even if it did that, that would still result in the caret being treated as inside the text input, which is not what we want.
3. If you change ToTextLeafPoint to instead return (HyperTextChild, 1), then the TextLeafPoint constructor will see that the document has children, walk to the first deepest descendant and use offset 0. That is, we end up with exactly the same result as we have now.
4. It's fairly easy to allow the TextLeafPoint constructor to accept (container, 1) to mean "end of container". The problem is that this breaks fundamental assumptions that TextLeafPoint makes. Just as one example, operator< will break because when the Accessibles are different, it simply uses Accessible::IsBefore to determine whether it's earlier, which will always return false because the document is always before any of its descendants.
5. This is only really a problem at the end of a document. So, maybe we just special case "end of document" in TextLeafPoint. That feels a bit ugly, but it could work.
6. Alternatively, maybe we just return no caret in this case. That's not strictly true, but the caret certainly isn't useful in this state.

Comment 10

[James Teh [:Jamie]]

This can also happen if the input is inside a large container and you click in the empty space inside the container after the input, even if there is text after the container. For example:
data:text/html,<div style="height: 50vh;"><input value="abc"></div>def
That makes this rather difficult to detect and compensate for. I'm reluctant to have ToTextLeafPoint simply return an invalid point for the end of a container because this could cause unexpected problems all over the place. On the other hand, GetCaret relies heavily on ToTextLeafPoint and doesn't have sufficient info to know that it started at the end of a container.

Comment 11

[James Teh [:Jamie]]

It can additionally happen if there is a large container, there is empty space in the container before an input and you click in the empty space before the input:
data:text/html,<div style="padding-top: 50vh;" tabindex="0"><input value="abc">
Weirdly, in this case, the caret offset we get from content is the last caret offset that was reported in the input, where I'd expect offset 0 in the div. So we can't detect this case in TextLeafPoint even if we wanted to; we'd need to figure out what's going on with caret calculation in content. This also means we'd need to special case a point at the start of a container, not just the end.

Comment 12

[James Teh [:Jamie]]

Attachment: Bug 1950748 part 1: HyperTextAccessibleBase::ToTextLeafPoint: Descend to the end of an embedded object when given its end offset.

Normally, an embedded object only spans 1 HyperText offset.
However, if the embedded object is the last child of its container, GetChildAtOffset(containerCharacterLength) will also return the last embedded object.
There are some rare cases where we end up with a HyperText offset that refers to the end of a container; e.g. the caret offset when clicking in the empty area at the bottom of a container after an input.
Previously, ToTextLeafPoint would descend to the start of the container in this case, which doesn't make any sense.
Instead, we now descend to the end.

In a subsequent patch for this bug, we will eliminate the case that triggered this behaviour.
However, I think this is still a correctness fix worth landing, as there may be other cases I'm not aware of yet.

Comment 13

[James Teh [:Jamie]]

Attachment: Bug 1950748 part 2: Remove some dead code and add an assertion.

As of bug 1901457, the TextLeafPoint constructor can no longer receive TEXT_OFFSET_CARET.
However, a related check was never removed from the constructor.
Remove this check and assert that this offset is never provided.
This is just a correctness fix that makes the code easier to reason about.

Comment 14

[James Teh [:Jamie]]

Attachment: Bug 1950748 part 3: Don't return a caret inside an editor when no editor is focused.

See the code comments for details.

Comment 15

[James Teh [:Jamie]]

I spent quite a bit of time trying to tweak TextLeafPoint to handle a point at the end of a non-empty container; i.e. (container, 1) even when container has children. The challenge is that containre is before its descendants in the tree, but we need points at the end of the container to be considered after descendants. This would require tweaks to operator<, FindBoundary, FindTextAttrsStart, TextLeafRange::Iterator and probably others I'm missing. For most of these, we can probably just check if we're at the end of a container - mOffset == 1 && mAcc->HasChildren() - and if so, normalise to the end of the container - TextLeafPoint(mAcc, nsIAccessibleText::TEXT_OFFSET_END_OF_TEXT)- and perform the operation on that normalised point. However, it's not so simple foroperator<` and there are possibly other cases I've missed. For reference, here's a patch which tweaks HyperTextAccessibleBase and TextLeafPoint to accept this position, though it doesn't make TextLeafPoint operations actually handle this correctly.

In addition, as I discovered in comment 11, this can also happen at the start of a container. The situation is worse there because we actually can't distinguish between HyperText offset 0 meaning "start of container" vs "first character of container". For TextLeafPoint, the fix is probably similar to the end of container case, except that we somehow need to tell the constructor not to descend to the leaf, since offset 0 is ambiguous in a non-empty container (unlike offset 1, which is currently invalid).

There's also the question of what character bounds to return for this start or end of container point, and likely other similar questions I haven't considered yet.

Given that this is only really a problem for the caret in this edge case, I decided it didn't make sense to try to support this in TextLeafPoint, with all the complexity and risk of regressions that would incur. Instead, the patches here simply return no caret in this case. As far as a client is concerned, the caret isn't useful in this case anyway.

Comment 16

[James Teh [:Jamie]]

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=141637

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/5565c5899c32
https://hg.mozilla.org/integration/autoland/rev/17243f4cab40
part 1: HyperTextAccessibleBase::ToTextLeafPoint: Descend to the end of an embedded object when given its end offset. r=morgan
https://github.com/mozilla-firefox/firefox/commit/f5f2e3cf4d34
https://hg.mozilla.org/integration/autoland/rev/a62a2d25a53f
part 2: Remove some dead code and add an assertion. r=morgan
https://github.com/mozilla-firefox/firefox/commit/aaf970807d28
https://hg.mozilla.org/integration/autoland/rev/43db53b9c173
part 3: Don't return a caret inside an editor when no editor is focused. r=morgan

Comment 18

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/17243f4cab40
https://hg.mozilla.org/mozilla-central/rev/a62a2d25a53f
https://hg.mozilla.org/mozilla-central/rev/43db53b9c173

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/80949017902d
https://hg.mozilla.org/mozilla-central/rev/57c30f1adca5
Revert "Bug 1950748 part 3: Don't return a caret inside an editor when no editor is focused. r=morgan" as requested for causing accessibility crashes (bug 1977012).

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e07922f34a4c
https://hg.mozilla.org/integration/autoland/rev/9ef5afc383e7
part 1: HyperTextAccessibleBase::ToTextLeafPoint: Descend to the end of an embedded object when given its end offset. r=morgan
https://github.com/mozilla-firefox/firefox/commit/9fc540cebb14
https://hg.mozilla.org/integration/autoland/rev/16cf34285142
part 2: Remove some dead code and add an assertion. r=morgan
https://github.com/mozilla-firefox/firefox/commit/655e5cc576ec
https://hg.mozilla.org/integration/autoland/rev/ebee26b8b8ce
part 3: Don't return a caret inside an editor when no editor is focused. r=morgan

Comment 21

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/9ef5afc383e7
https://hg.mozilla.org/mozilla-central/rev/16cf34285142
https://hg.mozilla.org/mozilla-central/rev/ebee26b8b8ce

Comment 22

[James Teh [:Jamie]]

Morgan, would you remind re-testing comment 0 and comment 6 with latest Nightly? These should hopefully be fixed now. Thanks.

Comment 23

[Morgan Reschenberg [:morgan]]

(In reply to Nathan LaPré from comment #0)

STR:

1. Turn on UIA support in about:config; set accessibility.uia.enable to true.
2. Turn on the Windows Text Cursor Indicator (Settings -> Accessibility -> Text Cursor -> Set Text cursor indicator to "On")
3. Open test page data:text/html,<input value="ABCDEF"/>
4. Using the mouse, click in the text box between the A and the B (to the right of the A but to the left of the B).
5. Click outside of the text box so that focus leaves the text input

Expected: Text cursor indicator disappears from the text input.
Actual: Text cursor indicator remains in the text input, and at an unexpected location.

This works in Edge; the text cursor indicator disappears. It somewhat works using the IA2 -> UIA bridge in Firefox, but it's broken in other, worse ways. The text cursor indicator should disappear, but also puzzling is that it remains in an unexpected and inconsistent location; sometimes at the very end, sometimes at the very start, sometimes somewhere in the middle. It seems to move to the start of the text for <input>s, and the end of the text for <div contenteditable>s. Weird. Regardless, this might be beside the point: I think the indicator should disappear, and it doesn't.

In latest nightly on windows 11:
Clicking between A and B correctly locates the text cursor between A and B.
Clicking outside of the text input causes the cursor to disappear.

Comment 24

[Morgan Reschenberg [:morgan]]

(In reply to James Teh [:Jamie] from comment #6)

(In reply to Morgan Reschenberg [:morgan] from comment #5)

FWIW if there is another element in content (like <button>hello or just hello), this problem doesn't occur.

Is this only the case if the other element is after the input? That is, if you put it before the input:

data:text/html,<button>hello</button><input value="abc">

I'm guessing you get the same bug if you click in the blank area of the document somewhere after the input?

This example functions the same as the classic input (no-button) example in C0; when you click outside the input, the text cursor disappears.

Comment 25

[James Teh [:Jamie]]

Excellent, thank you. I thus pronounce this fix verified.