1649005 - Crash when rewrapping an empty message via mozilla::HTMLEditor::InsertTextWithQuotationsInternal

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Patrick Brunschwig]

Applying Edit > Rewrap on an empty message leads to a crash.

Crash ID: bp-25b94efd-49db-419d-a050-0715d0200628

Comment 1

[Wayne Mery (:wsmwk)]

fairly rare. although some 78 crashes occur (and obviously don't have enigmail installed
bp-9a73e57a-113e-498d-be91-709300200624
bp-74995e92-b87b-48f6-8327-1360d0200623

Crash ID: bp-25b94efd-49db-419d-a050-0715d0200628 68.9.0

0 libxul.so mozilla::detail::nsTStringRepr<char16_t>::First() const xpcom/string/nsTSubstring.cpp:981
1 libxul.so mozilla::HTMLEditor::InsertTextWithQuotationsInternal(nsTSubstring<char16_t> const&) editor/libeditor/HTMLEditorDataTransfer.cpp:1846
2 libxul.so mozilla::HTMLEditor::Rewrap(bool) editor/libeditor/HTMLEditorDataTransfer.cpp:2104
3 libxul.so NS_InvokeByIndex
4 libxul.so XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) js/xpconnect/src/XPCWrappedNative.cpp:1157
5 libxul.so XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) js/xpconnect/src/XPCWrappedNativeJSOps.cpp:946
6 libxul.so js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) js/src/vm/Interpreter.cpp:535
7 libxul.so Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:594
8 libxul.so js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:423
9 libxul.so js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) js/src/vm/Interpreter.cpp:563
10 libxul.so <name omitted> js/src/vm/Interpreter.cpp:606
11 libxul.so JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) js/src/jsapi.cpp:2610
12 libxul.so nsXPCWrappedJS::CallMethod(unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJSClass.cpp:965
13 libxul.so PrepareAndDispatch xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:127
14 libxul.so SharedStub
15 libxul.so nsBaseCommandController::DoCommand(char const*) dom/commandhandler/nsBaseCommandController.cpp:115
16 libxul.so NS_InvokeByIndex
17 libxul.so XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) js/xpconnect/src/XPCWrappedNative.cpp:1157
18 libxul.so XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) js/xpconnect/src/XPCWrappedNativeJSOps.cpp:946
19 libxul.so js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) js/src/vm/Interpreter.cpp:535
20 libxul.so Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:594
21 libxul.so js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:423
22 libxul.so js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) js/src/vm/Interpreter.cpp:563
23 libxul.so <name omitted> js/src/vm/Interpreter.cpp:606
24 libxul.so JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) js/src/jsapi.cpp:2673
25 libxul.so mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) dom/bindings/EventHandlerBinding.cpp:267
26 libxul.so mozilla::JSEventHandler::HandleEvent(mozilla::dom::Event*) dom/events/JSEventHandler.cpp:205
27 libxul.so mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) dom/events/EventListenerManager.cpp:1044
28 libxul.so mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) dom/events/EventListenerManager.cpp:1239
29 libxul.so mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) dom/events/EventDispatcher.cpp:349
30 libxul.so mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) dom/events/EventDispatcher.cpp:551
31 libxul.so mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) dom/events/EventDispatcher.cpp:1047

Comment 2

[Wayne Mery (:wsmwk)]

Patrick can you still reproduce?

Signature has changed to mozilla::detail::nsTStringRepr<T>::First | mozilla::HTMLEditor::InsertTextWithQuotationsInternal
For example bp-9796443e-ac59-4b25-958d-21bbf0201009

Comment 3

[Patrick Brunschwig]

Yes, I can still reproduce it with TB 78.3.2: bp-529746e4-aff9-40cc-b71a-dc01a0201010

Steps to reproduce:

1. compose a message in plain-text mode (i.e. no HTML)
2. click into the message body field
3. select menu Edit -> Rewrap

Comment 4

[Wayne Mery (:wsmwk)]

Perfect - that matches the signature I just added. Thanks for confirming

Comment 5

[Magnus Melin [:mkmelin]]

Attachment: Bug 1649005 - fix crash when rewrapping an empty message via mozilla::HTMLEditor::InsertTextWithQuotationsInternal. r=masayuki

If empty input, aStringToInsert.First() will crash at https://searchfox.org/mozilla-central/rev/c2e3ac11be4837718c2604e58085fbc8252b69dd/xpcom/string/nsTSubstring.cpp#968

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7cca3c4602b
fix crash when rewrapping an empty message via mozilla::HTMLEditor::InsertTextWithQuotationsInternal. r=masayuki

Comment 7

[Razvan Maries]

Backed out for perma failures.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=KW58sfWfTLWnOcRZyBQZxg.0&resultStatus=testfailed%2Cbusted%2Cexception&revision=c7cca3c4602be07948880e812e5ed00834e2b18b

Logs:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=318351048&repo=autoland&lineNumber=2330
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=318350942&repo=autoland&lineNumber=1786

Backout: https://hg.mozilla.org/integration/autoland/rev/eb0a7bc80b59355bd975015560e533086ef338a7

Comment 8

[Patrick Brunschwig]

I believe this is for Magnus, not me?

Comment 9

[Magnus Melin [:mkmelin]]

Yes. Did some updates now, successful try here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ad2e6d2118585e681299112c07a213edb347c6d&selectedTaskRun=ekz6XXH_Q5Ktzkw0VFkcTQ.0

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b771ef58b06
fix crash when rewrapping an empty message via mozilla::HTMLEditor::InsertTextWithQuotationsInternal. r=masayuki

Comment 11

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/7b771ef58b06

Comment 12

[Magnus Melin [:mkmelin]]

Comment on attachment 9180914 [details]
Bug 1649005 - fix crash when rewrapping an empty message via mozilla::HTMLEditor::InsertTextWithQuotationsInternal. r=masayuki

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Crash fix
• User impact if declined: Will crash for a known sequence of actions
• Fix Landed on Version: 83
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Adds test to ensure crash doesn't happen.
• String or UUID changes made by this patch: none

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9180914 [details]
Bug 1649005 - fix crash when rewrapping an empty message via mozilla::HTMLEditor::InsertTextWithQuotationsInternal. r=masayuki

Simple crash fix with tests, approved for 78.5esr.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9180914 [details]
Bug 1649005 - fix crash when rewrapping an empty message via mozilla::HTMLEditor::InsertTextWithQuotationsInternal. r=masayuki

Actually, this needs a rebased patch for ESR78.

Comment 15

[Magnus Melin [:mkmelin]]

Rebased, and sent to try (successfully): https://treeherder.mozilla.org/jobs?repo=try&revision=7c0566440efa405275527f938c063a954985837c&selectedTaskRun=Llhuq2ezTrKQCjbVdn66Jw.0

Grab the patch from here: https://hg.mozilla.org/try/rev/24122a66c8ad8738fbf20c719143e83fc879f893, or how do I send an ESR version? The stuff that didn't apply was related to additional assertions that had been put in place after 78.

Comment 16

[Magnus Melin [:mkmelin]]

Comment on attachment 9180914 [details]
Bug 1649005 - fix crash when rewrapping an empty message via mozilla::HTMLEditor::InsertTextWithQuotationsInternal. r=masayuki

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Comment 12, crash fix.
• User impact if declined:
• Fix Landed on Version: 83
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String or UUID changes made by this patch: none

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9180914 [details]
Bug 1649005 - fix crash when rewrapping an empty message via mozilla::HTMLEditor::InsertTextWithQuotationsInternal. r=masayuki

Approved for 78.6esr, thanks.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/0322427df80a