1627924 - Crash in [@ mozilla::HTMLEditor::StartMoving]

Status: RESOLVED FIXED

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

Description

[Wayne Mery (:wsmwk)]

These crashes are only happening for Thunderbird (and SM). Earliest is TB68.1.2. All OS

This bug is for crash report bp-83132ca8-98db-4e17-bcbc-035d90200406.

Top 10 frames of crashing thread:

0 xul.dll mozilla::HTMLEditor::StartMoving editor/libeditor/HTMLAbsPositionEditor.cpp:356
1 xul.dll mozilla::HTMLEditor::OnMouseMove editor/libeditor/HTMLEditorObjectResizer.cpp:931
2 xul.dll mozilla::HTMLEditorEventListener::HandleEvent editor/libeditor/HTMLEditorEventListener.cpp:79
3 xul.dll mozilla::EventListenerManager::HandleEventSubType dom/events/EventListenerManager.cpp:1043
4 xul.dll mozilla::EventListenerManager::HandleEventInternal dom/events/EventListenerManager.cpp:1238
5 xul.dll mozilla::EventTargetChainItem::HandleEvent dom/events/EventDispatcher.cpp:349
6 xul.dll mozilla::EventTargetChainItem::HandleEventTargetChain dom/events/EventDispatcher.cpp:587
7 xul.dll mozilla::EventTargetChainItem::HandleEventTargetChain dom/events/EventDispatcher.cpp:631
8 xul.dll mozilla::EventDispatcher::Dispatch dom/events/EventDispatcher.cpp:1047
9 xul.dll mozilla::PresShell::EventHandler::DispatchEventToDOM layout/base/PresShell.cpp:8298

Comment 1

[Magnus Melin [:mkmelin]]

mGrabber should be null checked? https://searchfox.org/mozilla-central/rev/a707541ff423ade0d81cef6488e6ecfa09273886/editor/libeditor/HTMLAbsPositionEditor.cpp#368

Comment 2

[Firefox Bug Husbandry Bot]

Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is P3 (Backlog,) indicating it has been triaged, the bug's Severity is being updated to S3 (normal.)

Comment 3

[Wayne Mery (:wsmwk)]

(In reply to Magnus Melin [:mkmelin] from comment #1)

mGrabber should be null checked? https://searchfox.org/mozilla-central/rev/a707541ff423ade0d81cef6488e6ecfa09273886/editor/libeditor/HTMLAbsPositionEditor.cpp#368

jstutte, do you agree

Comment 4

[Jens Stutte [:jstutte]]

That code changed a lot since 68. Maybe Masayuki can help?

Comment 5

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

Yeah, must be easy to fix.

Comment 6

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

Attachment: testcase, move the red box to bottom-left, drop, and try to drag it again

Comment 7

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

Attachment: Bug 1627924 - Make `HTMLEditor::HideGrabberInternal()` reset dragging state r=emilio!

When the grabber to move absolutely positioned element is disabled,
HTMLEditor::HideGrabberInternal() is called to delete it. However,
it does not reset dragging state. Therefore, mousemove event listener
will try to handle drag even after the grabber is removed.

This patch makes HideGrabberInternal() reset the dragging state to
make the event listener stop handling the drag action.

However, I hit a buggy assertion in EventStateManager. It tries to
set active state to parent of the grabber (in this case, absolutely positioned
element). However, editable element in contenteditable cannot have
active state. Therefore, leaf becomes nullptr, but newleaf is the
absolutely positioned element. Therefore, this patch adds this condition
into the MOZ_ASSERT.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/27b2cbeb63c7
Make `HTMLEditor::HideGrabberInternal()` reset dragging state r=emilio

Comment 9

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/27b2cbeb63c7