Closed Bug 1276562 Opened 8 years ago Closed 8 years ago

Hang when inserting a bulleted or numbered list into a contentEditable

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox47 --- unaffected
firefox48 + fixed
firefox49 + verified

People

(Reporter: MarcoZ, Assigned: surkov)

Details

(Keywords: hang, regression)

Attachments

(1 file)

1. Start NVDA and Firefox.
2. Log into Gmail.
3. Start a new message.
4. In the body, press Ctrl+Shift+7 to insert a new numbered list.
5. Type the first letter.

Result: NVDA will say "Firefox (not responding)", and Firefox will hang for about two minutes, with the CPU fan spinning up. After about two minutes, it will stop spinning, Firefox will return to normal, and the typed letter will appear.

At least for me, there is no crash, but Jamie reported that, while he was testing it, he also got Firefox crashes, but without a CrashReporter dialog.

Note: After the un-freezing, entering text will actually push the bullet to the right, or so it apears in NVDA's view of the world. Afterwards, the list will appear fine, though. You can also now finish entering list items without further interruption.
Note that I also reproduced this in the WordPress WYSIWYG editor, which is TinyMCE, and this also uses a ContentEditable field for entering rich text.
Note that if you're not using braille (Marco is), you'll need to add a step 6 to Marco's STR:
6. Press NVDA+upArrow (laptop: nvda+l) to report the current line.

So, after step 5, the IAccessibleText for the list item object shows:
"x1. "
where "x" is the character you typed in step 5. The expected result is "1. x"
If you retrieve the line offsets for offset 0, you get (0, 4) as expected.
If you try to retrieve the line offsets at offset 1, you get a freeze/crash.
[Tracking Requested - why for this release]: Easily reproducible hang, sometimes even crash. Regression from some recent code reorganization, needs to be uplifted to 48 once fixed. Impact for both Firefox in ContentEditable situations as well as Thunderbird and SeaMonkey when someone inserts a bulleted or numbered list into an e-mail message and is using a screen reader.
Attached patch patchSplinter Review
Attachment #8757965 - Flags: review?(yzenevich)
(In reply to alexander :surkov from comment #4)
> Created attachment 8757965 [details] [diff] [review]
> patch

a try build https://archive.mozilla.org/pub/firefox/try-builds/surkov.alexander@gmail.com-9955df97ea596e0702222f03b944f429510fa0f1/

Marco, can you give it a try please if the patch helps?
(In reply to alexander :surkov from comment #5)
> Marco, can you give it a try please if the patch helps?
Yes, this fixes the bug. Thanks!
Tracking + for 48/49 based on the fact this is regression and bad user experience.
Comment on attachment 8757965 [details] [diff] [review]
patch

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

Looks good 1 nit and also logging should be commented out

::: accessible/html/HTMLListAccessible.cpp
@@ +96,5 @@
> +HTMLLIAccessible::InsertChildAt(uint32_t aIndex, Accessible* aChild)
> +{
> +  // Adjust index if there's a bullet.
> +  if (mBullet && aIndex == 0 && aChild != mBullet) {
> +    return HyperTextAccessible::InsertChildAt(1, aChild);

nit: i would prefer to have aIndex + 1 here instead of 1 so the logic could be followed easier.
Attachment #8757965 - Flags: review?(yzenevich) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3415732a219a1b1ab4db34c2c7c2ee893d32bba
Bug 1276562 - Hang when inserting a bulleted or numbered list into a contentEditable, r=yzen
https://hg.mozilla.org/mozilla-central/rev/b3415732a219
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Assignee: nobody → surkov.alexander
Comment on attachment 8757965 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1260862
[User impact if declined]: Long hangs or crashes when inserting bulleted or numbered lists into contentEditable like Gmail when a screen reader is running
[Describe test coverage new/current, TreeHerder]: Treeherder, Mochitest, manual test.
[Risks and why]: Low risk, keeps indexes in order.
[String/UUID change made/needed]: None.
Attachment #8757965 - Flags: approval-mozilla-aurora?
Verified fix in the 2016-06-02 Nightly build.
Comment on attachment 8757965 [details] [diff] [review]
patch

This patch fixes a regression - hang issue. Take it in aurora.
Attachment #8757965 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.