Closed Bug 1436216 Opened 7 years ago Closed 7 years ago

Too many warnings: '!mParent || mOffset.value() <= mParent->Length()' (EditorDOMPoint.h, line 373))

Categories

(Core :: DOM: Editor, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- unaffected
firefox59 --- fixed
firefox60 --- fixed

People

(Reporter: jorgk-bmo, Assigned: masayuki)

References

Details

(Keywords: regression)

Attachments

(2 files)

+++ This bug was initially created as a clone of Bug #1434969 +++ As per bug 1434969 comment #24, we see this 351 times in our Mozmill test suite, for example in our test composition/test-image-display.js (which is a short one). I changed the warning to MOZ_ASSERT() and here is the stack. Assertion failure: !mParent || mOffset.value() <= mParent->Length() (The offset is out of bounds), at c:\mozilla-source\comm-central\obj-i686-pc-mingw32\dist\include\mozilla/EditorDOMPoint.h:373 #01: mozilla::EditorDOMPointBase<nsINode *,nsIContent *>::Set (c:\mozilla-source\comm-central\obj-i686-pc-mingw32\dist\include\mozilla\editordompoint.h:373) #02: mozilla::EditorBase::CreateNode (c:\mozilla-source\comm-central\mozilla\editor\libeditor\editorbase.cpp:1479) #03: mozilla::EditorBase::DeleteSelectionAndCreateElement (c:\mozilla-source\comm-central\mozilla\editor\libeditor\editorbase.cpp:4558) #04: mozilla::HTMLEditor::InsertAsCitedQuotation (c:\mozilla-source\comm-central\mozilla\editor\libeditor\htmleditordatatransfer.cpp:1917) #05: nsMsgCompose::ConvertAndLoadComposeWindow (c:\mozilla-source\comm-central\mailnews\compose\src\nsmsgcompose.cpp:691) #06: QuotingOutputStreamListener::OnStopRequest (c:\mozilla-source\comm-central\mailnews\compose\src\nsmsgcompose.cpp:3005) For good measure I also ran composition\test-forwarded-eml-actions.js and it gives the same stack. To round it up I ran composition/test-send-format.js and that gave the same stack as well. So there's something fishy in HTMLEditor::InsertAsCitedQuotation() and below. Surely that doesn't run in Firefox, or does it? There is some setting and then the middle-mouse-button inserts a quote.
This is regression of bug 1425091. https://searchfox.org/mozilla-central/diff/d438423b799a8be481271ff62341aa8196df4276/editor/libeditor/EditorBase.cpp#1452 > - pointToInsert.Set(ret); > + pointToInsert.Set(ret, offset); Those lines are not same. That should've been changed as: > pointToInsert.Set(ret->GetParentNode(), offset); I'll post a patch with cleaning up the method to avoid unnecessary instance creation of EditorRawDOMPoint.
Assignee: nobody → masayuki
Blocks: 1425091
Status: NEW → ASSIGNED
OS: Unspecified → All
Hardware: Unspecified → All
Comment on attachment 8948959 [details] Bug 1436216 - EditorBase::CreateElement() should call RangeUpdater::SelAdjCreateNode() with point of new element https://reviewboard.mozilla.org/r/218382/#review224430 ::: editor/libeditor/EditorBase.cpp:1463 (Diff revision 1) > // we need to redesign mRangeUpdater as avoiding using indices. > - int32_t offset = static_cast<int32_t>(pointToInsert.Offset()); > + Unused << aPointToInsert.Offset(); > > AutoRules beginRulesSniffing(this, EditAction::createNode, nsIEditor::eNext); > > - nsCOMPtr<Element> ret; > + nsCOMPtr<Element> newElement; RefPtr<Element> newElement;
Attachment #8948959 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/46af948408ea EditorBase::CreateElement() should call RangeUpdater::SelAdjCreateNode() with point of new element r=m_kato
Attached patch Patch for Beta (deleted) — Splinter Review
Approval Request Comment [Feature/Bug causing the regression]: New regression of bug 1425091. [User impact if declined]: User may see oddd result when editing rich text editor. [Is this code covered by automated tests?]: Yes and no. Actual symptom isn't tested since we don't know how the regression actually affects user level. However, when we hit the case, we get warning of debug build. A lot of automated tests put this warning. [Has the fix been verified in Nightly?]: Not yet, but the lot of warnings have gone from automated test's log. [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?]: This just makes the method behave same as prior to bug 1425091. [String changes made/needed]: No.
Attachment #8949332 - Flags: approval-mozilla-beta?
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Comment on attachment 8949332 [details] [diff] [review] Patch for Beta Fix for a recent regression from 59. Let's uplift this for beta 9.
Attachment #8949332 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: