Closed Bug 1424450 Opened 6 years ago Closed 6 years ago

crash at null in [@ mozilla::HTMLEditRules::CreateStyleForInsertText]

Categories

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

59 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- fixed

People

(Reporter: tsmith, Assigned: m_kato)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(2 files)

Attached file testcase.html
==68853==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f845f17a43c bp 0x7ffe9a35ed70 sp 0x7ffe9a35ea40 T0)
==68853==The signal is caused by a READ memory access.
==68853==Hint: address points to the zero page.
    #0 0x7f845f17a43b in GetAsText /src/obj-firefox/dist/include/mozilla/dom/Text.h:46:10
    #1 0x7f845f17a43b in mozilla::HTMLEditRules::CreateStyleForInsertText(mozilla::dom::Selection&, nsIDocument&) /src/editor/libeditor/HTMLEditRules.cpp:4882
    #2 0x7f845f141e51 in mozilla::HTMLEditRules::WillInsertText(EditAction, mozilla::dom::Selection*, bool*, bool*, nsTSubstring<char16_t> const*, nsTSubstring<char16_t>*, int) /src/editor/libeditor/HTMLEditRules.cpp:1332:17
    #3 0x7f845f1410fd in mozilla::HTMLEditRules::WillDoAction(mozilla::dom::Selection*, mozilla::RulesInfo*, bool*, bool*) /src/editor/libeditor/HTMLEditRules.cpp:645:14
    #4 0x7f845f2a2687 in mozilla::TextEditor::InsertText(nsTSubstring<char16_t> const&) /src/editor/libeditor/TextEditor.cpp:714:24
    #5 0x7f845f1007b6 in mozilla::InsertPlaintextCommand::DoCommand(char const*, nsISupports*) /src/editor/libeditor/EditorCommands.cpp:1076:22
    #6 0x7f845d205a50 in nsControllerCommandTable::DoCommand(char const*, nsISupports*) /src/dom/commandhandler/nsControllerCommandTable.cpp:147:26
    #7 0x7f845d1fb926 in nsBaseCommandController::DoCommand(char const*) /src/dom/commandhandler/nsBaseCommandController.cpp:136:25
    #8 0x7f845d2024a4 in nsCommandManager::DoCommand(char const*, nsICommandParams*, mozIDOMWindowProxy*) /src/dom/commandhandler/nsCommandManager.cpp:212:22
    #9 0x7f845d736bdb in nsHTMLDocument::ExecCommand(nsTSubstring<char16_t> const&, bool, nsTSubstring<char16_t> const&, nsIPrincipal&, mozilla::ErrorResult&) /src/dom/html/nsHTMLDocument.cpp:3276:18
    #10 0x7f845cb1e8e1 in mozilla::dom::HTMLDocumentBinding::execCommand(JSContext*, JS::Handle<JSObject*>, nsHTMLDocument*, JSJitMethodCallArgs const&) /src/obj-firefox/dom/bindings/HTMLDocumentBinding.cpp:854:21
    #11 0x7f845cec0ad7 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /src/dom/bindings/BindingUtils.cpp:3042:13
    #12 0x7f846395f041 in CallJSNative /src/js/src/jscntxtinlines.h:291:15
    #13 0x7f846395f041 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:473
    #14 0x7f84639450a8 in CallFromStack /src/js/src/vm/Interpreter.cpp:528:12
    #15 0x7f84639450a8 in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3096
    #16 0x7f8463931810 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:423:12
    #17 0x7f846395f4ce in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:15
    #18 0x7f846395ffd2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:541:10
    #19 0x7f846445940c in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:3036:12
    #20 0x7f845c817cbe in mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/EventHandlerBinding.cpp:260:37
    #21 0x7f845d36aba3 in Call<nsISupports *> /src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:362:12
    #22 0x7f845d36aba3 in mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) /src/dom/events/JSEventHandler.cpp:215
    #23 0x7f845d331661 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1111:51
    #24 0x7f845d333572 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /src/dom/events/EventListenerManager.cpp:1286:20
    #25 0x7f845d31df5f in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:462:16
    #26 0x7f845d32184b in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:826:9
    #27 0x7f845f878281 in nsDocumentViewer::LoadComplete(nsresult) /src/layout/base/nsDocumentViewer.cpp:1070:7
    #28 0x7f8462be49c2 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /src/docshell/base/nsDocShell.cpp:7874:21
    #29 0x7f8462be08ea in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp:7667:7
    #30 0x7f8462be86ef in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp
    #31 0x7f8459be6247 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /src/uriloader/base/nsDocLoader.cpp:1319:3
    #32 0x7f8459be5451 in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /src/uriloader/base/nsDocLoader.cpp:862:14
    #33 0x7f8459be20e4 in nsDocLoader::DocLoaderIsEmpty(bool) /src/uriloader/base/nsDocLoader.cpp:751:9
    #34 0x7f8459be20fd in ChildDoneWithOnload /src/uriloader/base/nsDocLoader.h:204:9
    #35 0x7f8459be20fd in nsDocLoader::DocLoaderIsEmpty(bool) /src/uriloader/base/nsDocLoader.cpp:754
    #36 0x7f8459be411c in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /src/uriloader/base/nsDocLoader.cpp:633:5
    #37 0x7f8459be503c in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /src/uriloader/base/nsDocLoader.cpp
    #38 0x7f8457f082fa in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /src/netwerk/base/nsLoadGroup.cpp:629:28
    #39 0x7f845aeb2d67 in DoUnblockOnload /src/dom/base/nsDocument.cpp:9116:18
    #40 0x7f845aeb2d67 in nsDocument::UnblockOnload(bool) /src/dom/base/nsDocument.cpp:9038
    #41 0x7f845ea1f129 in nsBindingManager::DoProcessAttachedQueue() /src/dom/xbl/nsBindingManager.cpp:422:10
    #42 0x7f845ea85d94 in applyImpl<nsBindingManager, void (nsBindingManager::*)()> /src/obj-firefox/dist/include/nsThreadUtils.h:1142:12
    #43 0x7f845ea85d94 in apply<nsBindingManager, void (nsBindingManager::*)()> /src/obj-firefox/dist/include/nsThreadUtils.h:1148
    #44 0x7f845ea85d94 in mozilla::detail::RunnableMethodImpl<nsBindingManager*, void (nsBindingManager::*)(), true, (mozilla::RunnableKind)0>::Run() /src/obj-firefox/dist/include/nsThreadUtils.h:1192
    #45 0x7f8457d20d94 in mozilla::SchedulerGroup::Runnable::Run() /src/xpcom/threads/SchedulerGroup.cpp:396:25
    #46 0x7f8457d4766e in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1033:14
    #47 0x7f8457d633f0 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:508:10
    #48 0x7f8458bd599a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21
    #49 0x7f8458b2c929 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:326:10
    #50 0x7f8458b2c929 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:319
    #51 0x7f8458b2c929 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299
    #52 0x7f845ef7d5ca in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:157:27
    #53 0x7f846369218b in XRE_RunAppShell() /src/toolkit/xre/nsEmbedFunctions.cpp:865:22
    #54 0x7f8458b2c929 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:326:10
    #55 0x7f8458b2c929 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:319
    #56 0x7f8458b2c929 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299
    #57 0x7f8463691b7d in XRE_InitChildProcess(int, char**, XREChildData const*) /src/toolkit/xre/nsEmbedFunctions.cpp:691:34
    #58 0x4ee9f5 in content_process_main /src/browser/app/../../ipc/contentproc/plugin-container.cpp:63:30
    #59 0x4ee9f5 in main /src/browser/app/nsBrowserApp.cpp:280
    #60 0x7f84767a782f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
    #61 0x41e078 in _start (firefox+0x41e078)
Flags: in-testsuite?
Crash Signature: [@ mozilla::HTMLEditRules::CreateStyleForInsertText ]
Priority: -- → P1
Comment on attachment 8968816 [details]
Bug 1424450 - Don't set selection on ClearStyle.

https://reviewboard.mozilla.org/r/237548/#review243272

::: commit-message-789e3:13
(Diff revision 1)
> +Because SplitNodeTransation returns error since Selection::Collapse is failed.
> +Then, SplitNodeDeep in HTMLEditor::SplitStyleAvovePoint returns error.  But
> +this error is ignored.  So node will becomes null even if successful.
> +
> +CreateStyleForInsertText will set new selection when there is split node, so
> +we shouldn't selection on ClearStyle.

shouldn't _set_ selection?

::: commit-message-789e3:15
(Diff revision 1)
> +Also, InsertNodeTransation is ignored for error when Collapse is failed, but
> +SplitNodeTransaction isn't ignored.  We should create a rule when collapse is
> +failed on transaction.

Yeah, I think that transactions should not change selection automatically (except undo/redo). Then, we could remove AutoTransactionsConserveSelection. Then, the code becomes stateless and such code must be easier to read.

::: commit-message-789e3:19
(Diff revision 1)
> +
> +Also, InsertNodeTransation is ignored for error when Collapse is failed, but
> +SplitNodeTransaction isn't ignored.  We should create a rule when collapse is
> +failed on transaction.
> +
> +And at feature, we shouldn't set selection in CreateStyleForInsertText, and then, it should return new insertion point for InsertText instead of setting new

Wrap this long line.

::: editor/libeditor/HTMLEditRules.cpp:4942
(Diff revision 1)
>    while (item && node != rootElement) {
> -    NS_ENSURE_STATE(mHTMLEditor);
> +   // Transactions may set selection, but we will set selection if necessary.
> +   AutoTransactionsConserveSelection dontChangeMySelection(htmlEditor);

Looks like that this while loop should be wrapped with {} and AutoTransactionConserveSelection put before the while statement.  Then, we don't need to create and destroy its instaces repeatedly.
Attachment #8968816 - Flags: review?(masayuki) → review+
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/8aa0c90b5a3a
Don't set selection on ClearStyle. r=masayuki
https://hg.mozilla.org/mozilla-central/rev/8aa0c90b5a3a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Assignee: nobody → m_kato
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.