Crash in mozilla::HTMLEditRules::TryToJoinBlocks

RESOLVED FIXED in Firefox 58

Status

()

defect
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: marcia, Assigned: masayuki)

Tracking

({crash, regression, testcase-wanted})

Trunk
mozilla59
Unspecified
Windows 10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox57 unaffected, firefox58 fixed, firefox59 fixed)

Details

(crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is
report bp-0e9d9fe9-893b-42a7-a3a1-f0c1e0171114.
=============================================================

Crash seen in nightly, but also 58: http://bit.ly/2ms0OCa. Crashes in 58 started using 20171108110838.

Bug 1414713 landed in that timeframe - perhaps :masayuki can offer some insight. 

Top 10 frames of crashing thread:

0 xul.dll mozilla::HTMLEditRules::TryToJoinBlocks editor/libeditor/HTMLEditRules.cpp:2980
1 xul.dll mozilla::HTMLEditRules::WillDeleteSelection editor/libeditor/HTMLEditRules.cpp:2448
2 xul.dll mozilla::HTMLEditRules::WillDoAction editor/libeditor/HTMLEditRules.cpp:653
3 xul.dll mozilla::TextEditor::DeleteSelection editor/libeditor/TextEditor.cpp:663
4 xul.dll mozilla::EditorBase::HandleKeyPressEvent editor/libeditor/EditorBase.cpp:4912
5 xul.dll mozilla::HTMLEditor::HandleKeyPressEvent editor/libeditor/HTMLEditor.cpp:595
6 xul.dll mozilla::EditorEventListener::KeyPress editor/libeditor/EditorEventListener.cpp:608
7 xul.dll mozilla::EditorEventListener::HandleEvent editor/libeditor/EditorEventListener.cpp:418
8 xul.dll mozilla::EventListenerManager::HandleEventSubType dom/events/EventListenerManager.cpp:1118
9 xul.dll mozilla::EventListenerManager::HandleEventInternal dom/events/EventListenerManager.cpp:1293

=============================================================
Flags: needinfo?(masayuki)
Flags: needinfo?(masayuki)
Yeah, must be a regression of my work.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Flags: needinfo?(masayuki)
The assertion tells us an existing bug.

The bug is a regression of bug 1414713.
Blocks: 1392867, 1414713
Hmm, that's not a regression of bug 1392867. The comment in source code said that all list items in right list node should be moved to the left list node.  However, that has never done that actually at least since 2002...
https://searchfox.org/mozilla-central/rev/c187e71ad56f59a17262fe1007145fa1082d5967/editor/libeditor/html/nsHTMLEditRules.cpp#2578,2590-2591,2595,2624-2625,2630
No longer blocks: 1392867
If we know how to use this path, we can fix this bug correctly, though. Reaching the block is really complicated with current code...
Comment on attachment 8929801 [details]
Bug 1417492 - HTMLEditRules::TryToJoinBlocks() should no do nothing when left list node and right list node are not descendant of each other

https://reviewboard.mozilla.org/r/201004/#review206626
Attachment #8929801 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/6faac9d0daff
HTMLEditRules::TryToJoinBlocks() should no do nothing when left list node and right list node are not descendant of each other r=m_kato
https://hg.mozilla.org/mozilla-central/rev/6faac9d0daff
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment on attachment 8929801 [details]
Bug 1417492 - HTMLEditRules::TryToJoinBlocks() should no do nothing when left list node and right list node are not descendant of each other

Approval Request Comment
[Feature/Bug causing the regression]:
Regression of bug 1414713.

[User impact if declined]:
Nightly and Developer edition users may hit this MOZ_DIAGNOSTIC_ASSERT which was added for confirming we have dead code (if it's not dead code, nobody hits this assert).

[Is this code covered by automated tests?]:
No, we're still not sure how to reproduce this.

[Has the fix been verified in Nightly?]:
Not yet because we don't know how to reproduce this. However, this patch removes the dead code and MOZ_DIAGNOSTIC_ASSERT, so, this crash won't occur.

[Needs manual test from QE? If yes, steps to reproduce]:
No.

[List of other uplifts needed for the feature/fix]:
No.

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
Because this removes a for loop which is added in 2002 but haven't run due to almost impossible to make the continuation condition true due to using uninitialized variable. And just remove the MOZ_DIAGNOSTIC_ASSERT too.

[String changes made/needed]:
No.
Attachment #8929801 - Flags: approval-mozilla-beta?
Comment on attachment 8929801 [details]
Bug 1417492 - HTMLEditRules::TryToJoinBlocks() should no do nothing when left list node and right list node are not descendant of each other

Fix a regression of bug 1414713. Beta58+.
Attachment #8929801 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.