1619647 - Crash in [@ mozilla::WSRunObject::ReplacePreviousNBSPIfUnncessary]

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect, P2)

Description

[Calixte Denizet (:calixte)]

This bug is for crash report bp-791ab540-2916-429a-a6f8-4cc820200303.

Top 10 frames of crashing thread:

0 XUL mozilla::WSRunObject::ReplacePreviousNBSPIfUnncessary editor/libeditor/WSRunObject.cpp:1935
1 XUL mozilla::WSRunObject::InsertText editor/libeditor/WSRunObject.cpp:320
2 XUL mozilla::HTMLEditor::HandleInsertText editor/libeditor/HTMLEditSubActionHandler.cpp:1557
3 XUL mozilla::EditorBase::InsertTextAsSubAction editor/libeditor/EditorBase.cpp:5519
4 XUL mozilla::TextEditor::OnInputText editor/libeditor/TextEditor.cpp:411
5 XUL mozilla::HTMLEditor::HandleKeyPressEvent editor/libeditor/HTMLEditor.cpp:756
6 XUL mozilla::EditorEventListener::KeyPress editor/libeditor/EditorEventListener.cpp:551
7 XUL mozilla::HTMLEditorEventListener::HandleEvent editor/libeditor/HTMLEditorEventListener.cpp:98
8 XUL mozilla::EventListenerManager::HandleEventSubType dom/events/EventListenerManager.cpp:1079
9 XUL mozilla::EventListenerManager::HandleEventInternal dom/events/EventListenerManager.cpp:1271

There are 3 crashes (from 2 installations) in nightly 75 with buildid 20200303095030. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1618089.

[1] https://hg.mozilla.org/mozilla-central/rev?node=61f680923525

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Ah, simple mistake.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1619647 - Make `WSRunObject::ReplacePreviousNBSPIfUnnecessary()` check `IsEndOfContainer()` before calling `IsCharNBSP()` r=m_kato!

I forgot to add this check only here. (I also checked again for all similar
methods' callers.) So, if the point is end of a text node (i.e., offset equals
its length), IsCharNBSP() refers wrong address.

I cannot find a way to reproduce this crash, therefore, this patch does not
have new crashtest.

(Additionally, this corrects the misspell in the method name.)

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/01e8e25d291e
Make `WSRunObject::ReplacePreviousNBSPIfUnnecessary()` check `IsEndOfContainer()` before calling `IsCharNBSP()` r=m_kato

Comment 4

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/01e8e25d291e

Comment 5

[Catalin Sasca, Desktop QA [:csasca]]

Hi there Masayuki, is there something that QA can verify here? If so, could you please provide some steps? Thanks!

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Catalin Sasca, QA [:csasca] from comment #5)

Hi there Masayuki, is there something that QA can verify here? If so, could you please provide some steps? Thanks!

I have no idea. If I investigate with much time, I'll probably find it, but I don't have much time for this...