Closed Bug 1221043 Opened 9 years ago Closed 8 years ago

When entering text, spaces at end of lines are stripped in contentEditable text

Categories

(Core :: DOM: Core & HTML, defect)

x86
Windows 10
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla45
Tracking Status
firefox45 --- verified

People

(Reporter: MarcoZ, Assigned: roc)

References

Details

(Keywords: access, regression)

Attachments

(1 file)

This is a regression from bug 264412.

After the bug was fixed, when entering text into a contentEditable area, spaces at end of lines are stripped. Screen readers no longer detect that this is a space that was entered, but say "blank" instead. Also, braille displays no longer show a space character when entering one. Normal text fields and text areas are fine.

1. Open Gmail and start a new message.
2. Start entering text into the main message area. Type a word, then hit Space.
3. Now, turn on NVDA, and return to the main message area of Gmail.
4. Press LeftArrow.

Expected: NVDA should say "Space".
Actual: NVDA says "blank".

Also, with my connected refreshable braille display, there is no space between the blinking cursor and the last actual letter. So when I press left and right arrow over the last space character of the line, the blinking cursor position does not change.

Again: Regular inputs and textare aelements are fine, there, the space is not stripped. It is only contentEditable areas as common in rich text editors all across the web, where this happens.
This will be because GetRenderedText no longer returns a trailing space where whitespace is trimmed at the end of a line by CSS. <textarea> and <input> are not affected because they use white-space:pre so there is no trimming.

When updating the accessibility tests I found some "FIXME" cases where this change seemed to be desirable. I guess in this case it's not desirable.

For the visual caret, the trailing whitespace is trimmed, but we place the caret beyond the end of the line as if there was a space there. I guess accessibility could pass a flag to GetRenderedText telling it not to trim whitespace at the end of the line if the caret is there. Marco, how does that sound? Is it OK to continue to trim whitespace at the end of editable lines that do not have the caret at the end?
Flags: needinfo?(mzehe)
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #1)
> When updating the accessibility tests I found some "FIXME" cases where this
> change seemed to be desirable. I guess in this case it's not desirable.

Correct, because we are in an editing situation, not a browsing one. The WYSIWYG editor in Gmail and elsewhere is contentEditable.

> For the visual caret, the trailing whitespace is trimmed, but we place the
> caret beyond the end of the line as if there was a space there.

Is that easier than just letting a real whitespace guide the caret position in a contentEditable position? I am thinking of different fonts or font sizes where we'd get the correct daret position for free if we didn't trim the whitespace in an editable situation in general.

> I guess accessibility could pass a flag to GetRenderedText telling it not to trim
> whitespace at the end of the line if the caret is there. Marco, how does
> that sound? Is it OK to continue to trim whitespace at the end of editable
> lines that do not have the caret at the end?

I guess braille displays would be fine, as would screen readers. But I am not sure if magnifiers could be negatively affected by this by not giving an accurate representation of the line. Again, see my question in the previous paragraph: Wouldn't it be benefitial to have a true whitespace guide the caret position in a contenteditable situation like we do in inputs and textareas?
Flags: needinfo?(mzehe) → needinfo?(roc)
We always use the correct space width to place the caret, so that isn't a problem.

Whether there is "really" a space at the end of the line, or whether it's trimmed, is often not observable. But it is observable if the text is text-decoration:underline or a span with background-color. Per CSS, and as implemented in Firefox and Chrome at least, the trailing spaces of lines are trimmed and no background-color or text-decoration is drawn for them. And we don't want making elements contenteditable to affect layout and rendering.

So I think we should go with what I suggested.
Flags: needinfo?(roc)
Yeah let's try it. Should be OK. (famous last words... ;) )
Robert, can you do this? This seems to be layout code for the most part, implementing the flag and correct behavior etc.
Flags: needinfo?(roc)
Yes.
Assignee: nobody → roc
Flags: needinfo?(roc)
A problem with including trailing whitespace when the caret is at the end of a line is that we then have to update the accessibility tree every time the caret moves, which is tricky. Even worse, it may be a problem to have accessible text changing just because the caret moved.

To avoid issues here I'm just going to revert to the old state where accessibility APIs always get trailing whitespace.
Bug 1221043. Revert to including trailing whitespace for accessibility APIs. r=marcoz,mats
Attachment #8692703 - Flags: review?(mzehe)
Attachment #8692703 - Flags: review?(mats)
Comment on attachment 8692703 [details]
MozReview Request: Bug 1221043. Revert to including trailing whitespace for accessibility APIs. r=marcoz,mats

https://reviewboard.mozilla.org/r/26311/#review23763

r=me. I think this is indeed the least disruptive approach.
Attachment #8692703 - Flags: review?(mzehe) → review+
Attachment #8692703 - Flags: review?(mats) → review+
Comment on attachment 8692703 [details]
MozReview Request: Bug 1221043. Revert to including trailing whitespace for accessibility APIs. r=marcoz,mats

https://reviewboard.mozilla.org/r/26311/#review23793
https://hg.mozilla.org/mozilla-central/rev/ef68dc6a289d
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Verified fixed in the 2015-12-03 Nightly build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: