1794265 - Using GMail compose with Firefox Nightly and screen readers freezes browser

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect, P1)

Description

[4RJames]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:107.0) Gecko/20100101 Firefox/107.0

Steps to reproduce:

While using GMail with Firefox Nightly
This started happening a few days ago...
Using the NVDA or Narator screen readers
Compose a new message
When you start to edit the message body in the edit box Firefox Nightly freezes
When you type characters they are not displayed or spoken by the screen reader

Actual results:

The GMail compose window remains on the display but there are no updates
The sound effects and spoken feedback are silent
When focus is moved to that edit box there is no audible click
When you type characters they don't appear and are not spoken
You can no longer interact with that tab/window
The browser or computer must be restarted to recover
Task manager often reports Firefox Nightly taking 10% CPU

Expected results:

Normally you can compose the email message in the edit box
When the characters are typed they are displayed and spoken

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Disability Access APIs' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[4RJames]

Sorry, I neglected to indicate this happens every time.
It also still works correctly on regular Firefox.

Comment 3

[James Teh [:Jamie]]

[Tracking Requested - why for this release]: Gmail composer freezes Firefox when using the NVDA screen reader.

Ouch. I can confirm this. Curiously, it doesn't happen with the new cache enabled.

Comment 4

[James Teh [:Jamie]]

6:49.82 INFO: Last good revision: 561d75b5153b0a7b7c8a4dfb064ffcd477da2b1d
6:49.82 INFO: First bad revision: 66a061bd76fc63ce67845e4f848a3bd8cd9b703d
6:49.82 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=561d75b5153b0a7b7c8a4dfb064ffcd477da2b1d&tochange=66a061bd76fc63ce67845e4f848a3bd8cd9b703d
This implicates bug 1792333 (specifically, "Various clean-ups to nsIFrame line-movement APIs.").

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1792333

Comment 6

[James Teh [:Jamie]]

Here's a stack when the freeze occurred:

# Child-SP          RetAddr           Call Site
00 (Inline Function) --------`-------- xul!nsIFrame::GetOffsetFromView(struct nsPoint * aOffset = <Value unavailable error>, class nsView ** aView = <Value unavailable error>)+0x13 [C:\Users\jamie\src\gecko2\layout\generic\nsIFrame.cpp @ 7115]
01 00000044`405fd570 00007ff9`e18cf077 xul!GetNextPrevLineFromBlockFrame(struct nsPeekOffsetStruct * aPos = 0x00000044`405fd7b8, class nsIFrame * aBlockFrame = 0x0000017e`2e618d20, int aLineStart = <Value unavailable error>, char aOutSideLimit = <Value unavailable error>)+0x223 [C:\Users\jamie\src\gecko2\layout\generic\nsIFrame.cpp @ 8387]
02 00000044`405fd6b0 00007ff9`e27ee740 xul!nsIFrame::PeekOffsetForLine(struct nsPeekOffsetStruct * aPos = 0x00000044`405fd7b8)+0xe7 [C:\Users\jamie\src\gecko2\layout\generic\nsIFrame.cpp @ 8962]
03 00000044`405fd740 00007ff9`e27ef058 xul!mozilla::a11y::HyperTextAccessible::FindOffset(void)+0x2c0 [C:\Users\jamie\src\gecko2\accessible\generic\HyperTextAccessible.cpp @ 535]
04 00000044`405fd8f0 00007ff9`e27efebd xul!mozilla::a11y::HyperTextAccessible::FindLineBoundary(unsigned int aOffset = 0, mozilla::a11y::HyperTextAccessible::EWhichLineBoundary aWhichLineBoundary = <Value unavailable error>)+0x448 [C:\Users\jamie\src\gecko2\accessible\generic\HyperTextAccessible.cpp @ 738]
05 00000044`405fda00 00007ff9`e279f91f xul!mozilla::a11y::HyperTextAccessible::TextAtOffset(int aOffset = 0n0, int aBoundaryType = 0n5, int * aStartOffset = 0x00000044`405fdb4c, int * aEndOffset = 0x00000044`405fdb48, class nsTSubstring<char16_t> * aText = 0x00000044`405fdb50 "")+0x23d [C:\Users\jamie\src\gecko2\accessible\generic\HyperTextAccessible.cpp @ 1064]
06 00000044`405fdb10 00007ffa`47e54dbe xul!mozilla::a11y::ia2AccessibleText::get_textAtOffset(long aOffset = 0n0, IA2TextBoundaryType aBoundaryType = <Value unavailable error>, long * aStartOffset = 0x00000044`4427e150, long * aEndOffset = 0x00000044`4427e160, wchar_t ** aText = 0x00000044`4427e170)+0x14f [C:\Users\jamie\src\gecko2\accessible\windows\ia2\ia2AccessibleText.cpp @ 322]
...

A second or so later, I took another stack:

 # Child-SP          RetAddr           Call Site
00 (Inline Function) --------`-------- xul!nsFrameIterator::nsFrameIterator(class nsPresContext * aPresContext = 0x0000017e`2b546000, class nsIFrame * aStart = 0x0000017e`2e61c020, nsIteratorType aType= ePostOrder (0n2), bool aLockInScrollView = false, bool aFollowOOFs = false, bool aSkipPopupChecks = false)+0x32 [C:\Users\jamie\src\gecko2\layout\base\nsFrameTraversal.cpp @ 192]
01 00000044`405fd4f0 00007ff9`e18cf576 xul!NS_NewFrameTraversal(class nsIFrameEnumerator ** aEnumerator = 0x00000044`405fd620, class nsPresContext * aPresContext = 0x0000017e`2b546000, class nsIFrame *aStart = 0x0000017e`2e61c020, nsIteratorType aType = ePostOrder (0n2), bool aVisual = <Value unavailable error>, bool aLockInScrollView = <Value unavailable error>, bool aFollowOOFs = <Value unavailable error>, bool aSkipPopupChecks = <Value unavailable error>)+0xf4 [C:\Users\jamie\src\gecko2\layout\base\nsFrameTraversal.cpp @ 153]
02 00000044`405fd570 00007ff9`e18cf0ff xul!GetNextPrevLineFromBlockFrame(struct nsPeekOffsetStruct * aPos = 0x00000044`405fd7b8, class nsIFrame * aBlockFrame = 0x0000017e`2e618d20, int aLineStart = <Value unavailable error>, char aOutSideLimit = <Value unavailable error>)+0x2f6 [C:\Users\jamie\src\gecko2\layout\generic\nsIFrame.cpp @ 8408]
03 00000044`405fd6b0 00007ff9`e27ee740 xul!nsIFrame::PeekOffsetForLine(struct nsPeekOffsetStruct * aPos = 0x00000044`405fd7b8)+0x16f [C:\Users\jamie\src\gecko2\layout\generic\nsIFrame.cpp @ 8959]
04 00000044`405fd740 00007ff9`e27ef058 xul!mozilla::a11y::HyperTextAccessible::FindOffset(void)+0x2c0 [C:\Users\jamie\src\gecko2\accessible\generic\HyperTextAccessible.cpp @ 535]
05 00000044`405fd8f0 00007ff9`e27efebd xul!mozilla::a11y::HyperTextAccessible::FindLineBoundary(unsigned int aOffset = 0, mozilla::a11y::HyperTextAccessible::EWhichLineBoundary aWhichLineBoundary = <Value unavailable error>)+0x448 [C:\Users\jamie\src\gecko2\accessible\generic\HyperTextAccessible.cpp @ 738]
06 00000044`405fda00 00007ff9`e279f91f xul!mozilla::a11y::HyperTextAccessible::TextAtOffset(int aOffset = 0n0, int aBoundaryType = 0n5, int * aStartOffset = 0x00000044`405fdb4c, int * aEndOffset = 0x00000044`405fdb48, class nsTSubstring<char16_t> * aText = 0x00000044`405fdb50 "")+0x23d [C:\Users\jamie\src\gecko2\accessible\generic\HyperTextAccessible.cpp @ 1064]
...

This suggests we're stuck in an infinite loop in GetNextPrevLineFromBlockFrame.

Comment 7

[James Teh [:Jamie]]

I can't yet figure out what's special about the Gmail composer. It looks like a contenteditable containing a <br>, but <div contenteditable><br></div> isn't enough to cause this hang.

Comment 8

[James Teh [:Jamie]]

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

Curiously, it doesn't happen with the new cache enabled.

The cache uses TextLeafPoint, which does still care about lines but doesn't use PeekOffset any more. Somehow, that makes it immune to this bug.

Comment 9

[James Teh [:Jamie]]

Emilio, any idea what's going on here?

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Not off-hand, but can investigate!

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: Reduced test-case.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1794265 - Fix infinite loop in PeekOffsetFromLine. r=Jamie,dholbert

This effectively restores the blockFrame != aPos->mResultFrame check
here:

https://hg.mozilla.org/integration/autoland/rev/66a061bd76fc63ce67845e4f848a3bd8cd9b703d#l1.260

This is hitting this rather odd code-path from the netscape era, which
is clearly not well tested:

https://searchfox.org/mozilla-central/rev/ffa4d00965c5281def6d3ddcbcdf6259d38c9b9a/layout/generic/nsIFrame.cpp#8451-8482

I'd happily write a test, but I think I'd need some assistance as I
haven't been able to reproduce this with regular selection APIs / line
movement.

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1794265 - Remove untested, Netscape-era editor hack. r=masayuki,dholbert

Behavior of an editor with only a table seems the same before and after
this patch when positioning the caret with keyboard or mouse, which
seems what this was about (see bug 34356). Nobody reads the magic return
value either, so remove it as well.

If we wanted to tweak this, there's no reason to special-case tables
(we'd want to allow positioning in any arbitrary line-iterable frame).

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Jamie, any chance you could help me figure out how to write an automated test for this? I couldn't repro this with regular Selection.modify calls or so, which is how I'd usually go about these.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d7103c5e7e0c
Fix infinite loop in PeekOffsetFromLine. r=dholbert

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8613dae8d38b
Remove untested, Netscape-era editor hack. r=dholbert

Comment 17

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/d7103c5e7e0c
https://hg.mozilla.org/mozilla-central/rev/8613dae8d38b

Comment 18

[James Teh [:Jamie]]

I can reproduce this by calling HyperTextAccessible::TextAtOffset on the editor root, passing 0 as the offset and nsIAccessibleText::BOUNDARY_LINE_START. As to how to do this in a test, I'd probably add something to accessible/tests/mochitest/text/test_lineboundary.html.

testTextAtOffset("your_new_id", BOUNDARY_LINE_START, [
  [0, 0, kEmbedChar, 0, 1],
]);

I'm a little confused by the reduced test case, as it deals with a table inside an editor, but from what I could see, the Gmail composer was an editor containing only a <br>. That said, I can definitely reproduce the hang with your test case, so that probably doesn't matter.

Comment 19

[Emilio Cobos Álvarez (:emilio)]

The table in GMail is a pseudo-element fwiw. Ok, I can try to write a test.

Comment 20

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1794265 - Add a test for this bug. r=Jamie,morgan

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07d359a116b5
Add a test for this bug. r=Jamie

Comment 22

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/07d359a116b5

Comment 23

[BugBot [:suhaib / :marco/ :calixte]]

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

Comment 24

[Virgil Sangerean]

Reproduced on Windows 10 with build from 8th of October 2022. Did not manage to reproduce issue on Mac Nor Linux from same date.

Verified fixed on Nightly from 24 October 2022.