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

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[ISHIKAWA, Chiaki]

Attachment: I am attaching a one test case where both warnings mentioned in the bugzilla appear.

TB was built locally from the C-C tree fetched about a week ago.

This is a full debug build with -DDEBUG defined during compilation.

When |make mozmill| test suite was run, I notice there are many warnings of the form as follows.


[7755, Main Thread] WARNING: 'mOffset.value() >= mParent->Length()', file /NREF-COMM-CENTRAL/objdir-tb3/dist/include/mozilla/EditorDOMPoint.h, line 500

and 

[7624, Main Thread] WARNING: The offset is out of bounds: '!mParent || mOffset.value() <= mParent->Length()', file /NREF-COMM-CENTRAL/objdir-tb3/dist/include/mozilla/EditorDOMPoint.h, line 373

Actually, the former outnumbers the latter by a factor of approximately 6.

The following is the list of WARNING lines sorted based on the # of frequencies.
The first PID part in the prefix "[PID, Main Thread]" part is omitted for the sake of comparison.
I show only the top 20 entries.

The number at the beginning of the line is the # of appearances.
I mark the warning in question with a "*" at the beginning of the line.

* 2289: Main Thread] WARNING: 'mOffset.value() >= mParent->Length()', file /NREF-COMM-CENTRAL/objdir-tb3/dist/include/mozilla/EditorDOMPoint.h, line 500
  1971: Main Thread] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/base/nsTraceRefcnt.cpp, line 171
  1376: Main Thread] WARNING: '!mMainThread', file /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/nsThreadManager.cpp, line 403
  1088: Main Thread] WARNING: NS_ENSURE_TRUE(ps) failed: file /NREF-COMM-CENTRAL/comm-central/mozilla/editor/libeditor/HTMLEditor.cpp, line 2872
  1048: Main Thread] WARNING: Failed to retarget HTML data delivery to the parser thread.: file /NREF-COMM-CENTRAL/comm-central/mozilla/parser/html/nsHtml5StreamParser.cpp, line 1009
  584: Main Thread] WARNING: NS_ENSURE_TRUE(aNode) failed: file /NREF-COMM-CENTRAL/comm-central/mozilla/editor/libeditor/EditorBase.cpp, line 3904
* 350: Main Thread] WARNING: The offset is out of bounds: '!mParent || mOffset.value() <= mParent->Length()', file /NREF-COMM-CENTRAL/objdir-tb3/dist/include/mozilla/EditorDOMPoint.h, line 373
  307: Main Thread] WARNING: '!mTextEditor', file /NREF-COMM-CENTRAL/comm-central/mozilla/extensions/spellcheck/src/mozInlineSpellChecker.cpp, line 724
  307: Main Thread] WARNING: '!mTextEditor', file /NREF-COMM-CENTRAL/comm-central/mozilla/extensions/spellcheck/src/mozInlineSpellChecker.cpp, line 1730
  306: Main Thread] WARNING: '!aPoint.IsSetAndValid()', file /NREF-COMM-CENTRAL/comm-central/mozilla/editor/libeditor/SelectionState.cpp, line 227
  289: Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80550006: file /NREF-COMM-CENTRAL/comm-central/mailnews/local/src/nsMailboxService.cpp, line 672
  289: Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80550006: file /NREF-COMM-CENTRAL/comm-central/mailnews/base/util/nsMsgDBFolder.cpp, line 5134
  272: Main Thread] WARNING: NS_ENSURE_TRUE(sheet) failed: file /NREF-COMM-CENTRAL/comm-central/mozilla/editor/libeditor/HTMLEditor.cpp, line 2866
  226: Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80070057: file /NREF-COMM-CENTRAL/comm-central/mozilla/netwerk/base/nsChannelClassifier.cpp, line 344
  170: Main Thread] WARNING: We should have hit the document element...: file /NREF-COMM-CENTRAL/comm-central/mozilla/layout/xul/BoxObject.cpp, line 174
  162: Main Thread] WARNING: NS_ENSURE_TRUE(ioService) failed: file /NREF-COMM-CENTRAL/comm-central/mailnews/base/util/nsMsgMailNewsUrl.cpp, line 622
  162: Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8000FFFF: file /NREF-COMM-CENTRAL/comm-central/mailnews/local/src/nsMailboxUrl.cpp, line 181
  152: Main Thread] WARNING: NS_ENSURE_TRUE(msgDocShellItem) failed: file /NREF-COMM-CENTRAL/comm-central/mailnews/base/src/nsMsgWindow.cpp, line 90
  148: Main Thread] WARNING: NS_ENSURE_TRUE(name) failed: file /NREF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsDOMAttributeMap.cpp, line 327
  67: Main Thread] WARNING: unable to post continuation event: file /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/io/nsStreamUtils.cpp, line 485


The second and the third most frequent messages, i.e., the following two have been in the log for a long time.

  1971: Main Thread] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /NREF-COMM-CENTRAL/comm-central/mozilla/xpcom/base/nsTraceRefcnt.cpp, line 171
  1376: Main Thread] WARNING: '!mMainThread', file /NREF-COMM-CENTRAL/comm- 

The two warnings mentioned in this bugzilla are related to Editor.
Coupled with other warnings related to Editor, I suspect there are some serious issues such as the data object expected by Editing environment may not be present after a startup, etc.

Anyway, I *think* the warnings were not there last summer and late fall when I tested local build of C-C  TB. Busy day time job kept me from tinkering with C-C code until now since then.

I am attaching a single test case where the two warnings related to mparent->Length() appear.
But the warnings appear in many tests basically. Something is incorrect with EditorDom setup or the assumption does not hold for C-C TB.


TIA

Comment 1

[ISHIKAWA, Chiaki]

I am requesting information from Masayuki san as per Jorg's idea.
See:
https://groups.google.com/forum/#!topic/mozilla.dev.apps.thunderbird/KpPIu5eRCcY

TIA

Comment 2

[Jorg K (CEST = GMT+2)]

I'm taking the liberty to adjust the subject and to move the bug to Code::Editor. Perhaps these warnings could be toned down or their root cause addressed.

Comment 3

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

The warning means that EditorDOMPoint is set, then, the DOM tree is change, and finally, editor tries to rewind the offset. So, editor refers outdated DOM point. I.e., this is obviously an editor's bug.

Could you get a stack trace when you hit the warning? Then, I could fix it easy.

Comment 4

[Jorg K (CEST = GMT+2)]

I'd have to check which test produces the error and then see what the test does and repeat that action manually to get you a stack, unless Chiaki-san beats me to it.

Comment 5

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

Perhaps, just inserting MOZ_ASSERT(mOffset.value() < mParent->Length()); into the if block can show stack quickly.

Comment 6

[Jorg K (CEST = GMT+2)]

This one |'mOffset.value() >= mParent->Length()' (EditorDOMPoint.h, line 500)| shows up if you start a new message in TB, that is, open a new compose window. Stack:

xul.dll!mozilla::EditorDOMPointBase<nsINode *,nsIContent *>::RewindOffset() Line 502	C++
xul.dll!mozilla::HTMLEditRules::PromoteRange(nsRange & aRange, mozilla::EditAction aOperationType) Line 6087	C++
xul.dll!mozilla::HTMLEditRules::AfterEditInner(mozilla::EditAction aAction, short aDirection) Line 478	C++
xul.dll!mozilla::HTMLEditRules::AfterEdit(mozilla::EditAction aAction, short aDirection) Line 416	C++
xul.dll!mozilla::HTMLEditor::EndOperation() Line 3342	C++
xul.dll!mozilla::AutoRules::~AutoRules() Line 397	C++
xul.dll!mozilla::HTMLEditor::DoInsertHTMLWithContext(const nsTSubstring<char16_t> & aInputString, const nsTSubstring<char16_t> & aContextStr, const nsTSubstring<char16_t> & aInfoStr, const nsTSubstring<char16_t> & aFlavor, nsIDOMDocument * aSourceDoc, nsIDOMNode * aDestNode, int aDestOffset, bool aDeleteSelection, bool aTrustedInput, bool aClearStyle) Line 217	C++
xul.dll!mozilla::HTMLEditor::InsertHTMLWithContext(const nsTSubstring<char16_t> & aInputString, const nsTSubstring<char16_t> & aContextStr, const nsTSubstring<char16_t> & aInfoStr, const nsTSubstring<char16_t> & aFlavor, nsIDOMDocument * aSourceDoc, nsIDOMNode * aDestNode, int aDestOffset, bool aDeleteSelection) Line 191	C++
xul.dll!mozilla::HTMLEditor::InsertHTML(const nsTSubstring<char16_t> & aInString) Line 176	C++
xul.dll!nsMsgCompose::ConvertAndLoadComposeWindow(nsTString<char16_t> & aPrefix, nsTString<char16_t> & aBuf, nsTString<char16_t> & aSignature, bool aQuoted, bool aHTMLEditor) Line 761	C++
xul.dll!nsMsgCompose::BuildBodyMessageAndSignature() Line 4676	C++
xul.dll!nsMsgCompose::InitEditor(nsIEditor * aEditor, mozIDOMWindowProxy * aContentWindow) Line 1666	C++

Of course our tests create a few messages ;-)

The other one comes soon after:

xul.dll!mozilla::EditorDOMPointBase<nsCOMPtr<nsINode>,nsCOMPtr<nsIContent> >::Set(nsINode * aContainer, int aOffset) Line 372	C++
xul.dll!mozilla::HTMLEditor::InsertNodeIntoProperAncestor(nsIContent & aNode, const mozilla::EditorDOMPointBase<nsINode *,nsIContent *> & aPointToInsert, mozilla::SplitAtEdges aSplitAtEdges) Line 1579	C++
xul.dll!mozilla::HTMLEditor::InsertElementAtSelection(nsIDOMElement * aElement, bool aDeleteSelection) Line 1486	C++
xul.dll!_NS_InvokeByIndex() Line 57	Unknown

_NS_InvokeByIndex() indicates that something is run from JS, in this case the JS stack is:

0 NotifyComposeBodyReadyNew() ["chrome://messenger/content/messengercompose/MsgComposeCommands.js":437]
    this = [object Object]
1 NotifyComposeBodyReady() ["chrome://messenger/content/messengercompose/MsgComposeCommands.js":352]
    this = [object Object]
2 InitEditor() ["chrome://messenger/content/messengercompose/MsgComposeCommands.js":6495]
    this = [object ChromeWindow]
3 observe(aSubject = [xpconnect wrapped nsICommandManager @ 0x193c99a0 (native @
 0x20feefc0)], aTopic = "obs_documentCreated", aData = "command_status_changed")
 ["chrome://messenger/content/messengercompose/MsgComposeCommands.js":3004]
    this = [object Object]

MsgComposeCommands.js:437 inserts a paragraph that contains a break:
      editor.selection.collapse(mailBody, 0);
      let pElement = editor.createElementWithDefaults("p");
      pElement.appendChild(editor.createElementWithDefaults("br"));
437   editor.insertElementAtSelection(pElement, false);

Let me know if you want me to do further debugging. Note: I didn't look at the Mozmill logs, I just started a local debug build and created a new message et voilà.

Comment 7

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

(In reply to Jorg K (GMT+1) from comment #6)
> xul.dll!mozilla::EditorDOMPointBase<nsINode *,nsIContent *>::RewindOffset()
> Line 502	C++
> xul.dll!mozilla::HTMLEditRules::PromoteRange(nsRange & aRange,
> mozilla::EditAction aOperationType) Line 6087	C++
> xul.dll!mozilla::HTMLEditRules::AfterEditInner(mozilla::EditAction aAction,
> short aDirection) Line 478	C++

This might be bug of EditorDOMPointBase<>::RewindOffset(). Looks like that offset of it must be fine according to PromoteRange() and GetPromotedPoint(). mOffset.value() should be fine even if it equals length of its container because such point means after the last child.

> The other one comes soon after:
> 
> xul.dll!mozilla::EditorDOMPointBase<nsCOMPtr<nsINode>,nsCOMPtr<nsIContent>
> >::Set(nsINode * aContainer, int aOffset) Line 372	C++
> xul.dll!mozilla::HTMLEditor::InsertNodeIntoProperAncestor(nsIContent &
> aNode, const mozilla::EditorDOMPointBase<nsINode *,nsIContent *> &
> aPointToInsert, mozilla::SplitAtEdges aSplitAtEdges) Line 1579	C++

Hmm, I don't understand this stack yet... #1579 of HTMLEditor.cpp is the end of IsertNodeIntoProperAncestor() but I don't understand why Set() is called from here.

> xul.dll!mozilla::HTMLEditor::InsertElementAtSelection(nsIDOMElement *
> aElement, bool aDeleteSelection) Line 1486	C++
> xul.dll!_NS_InvokeByIndex() Line 57	Unknown
> 
> _NS_InvokeByIndex() indicates that something is run from JS, in this case
> the JS stack is:
> 
> 0 NotifyComposeBodyReadyNew()
> ["chrome://messenger/content/messengercompose/MsgComposeCommands.js":437]
>     this = [object Object]

I have a question. Do you know whether Thunderbird's composer uses mutation observer or mutation events? I mean that if Thunderbird observes DOM tree change of editor and it modifies the result of edit action, can cause this kind of warnings.

Anyway, I should fix the first point.

Comment 8

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3fd91367342e137ee98098d27199bce8ec15115e

Comment 9

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

Attachment: Bug 1434969 - EditorDOMPointBase::RewindOffset() shouldn't treat it as error if offset is same as length of the container

EditorDOMPointBase::RewindOffset() warns when offset is same or gratner
than lengh of container for detecting existing bugs.  However, this is
wrong because offset can be same as length of its container since it
means after the last child.

Review commit: https://reviewboard.mozilla.org/r/217740/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/217740/

Comment 10

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d0f625e3dee5d52ecc04be9a2eeccb19fc2af8f7

Comment 11

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

[Tracking Requested - why for this release]:

This is regression of bug 1419745. And this may break something of editor (if the point is set at end of a container element, moving the point to before the last child fails). We should uplift it.

Comment 12

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

Comment on attachment 8948164 [details]
Bug 1434969 - EditorDOMPointBase::RewindOffset() shouldn't treat it as error if offset is same as length of the container

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/217740/diff/1-2/

Comment 13

[Jorg K (CEST = GMT+2)]

Thanks for the quick action. Please not the spelling errors "gratner" and "lengh" in your patch.

As for your question, yes, we use mutation observers, but only for the lang attribute, and we don't modify the DOM in response:
https://dxr.mozilla.org/comm-central/rev/e62f73758e138743a47f2682f721862f09cd89c4/mail/components/compose/content/MsgComposeCommands.js#2607

For the second point I can understand better what's going on than for the first.

When you start a new e-mail you get a new windows and a new editor inside it. We put all sorts of things into the editor in C++ code, like a signature, and when done, the C++ code calls NotifyComposeBodyReadyNew() from here:
https://dxr.mozilla.org/comm-central/rev/e62f73758e138743a47f2682f721862f09cd89c4/mailnews/compose/src/nsMsgCompose.cpp#1666

That comes into the JS code in MsgComposeCommands.js and inserts a paragraph. As you know, TB now defaults into "paragraph mode".
https://dxr.mozilla.org/comm-central/rev/e62f73758e138743a47f2682f721862f09cd89c4/mail/components/compose/content/MsgComposeCommands.js#437

So something triggers the warning when inserting the paragraph via InsertElementAtSelection().

Comment 14

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

Comment on attachment 8948164 [details]
Bug 1434969 - EditorDOMPointBase::RewindOffset() shouldn't treat it as error if offset is same as length of the container

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/217740/diff/2-3/

Comment 15

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

(In reply to Jorg K (GMT+1) from comment #13)
> Thanks for the quick action. Please not the spelling errors "gratner" and
> "lengh" in your patch.

Thanks! I used an environment which I don't use usually. So, I wrote it without spellchecker.

> As for your question, yes, we use mutation observers, but only for the lang
> attribute, and we don't modify the DOM in response:
> https://dxr.mozilla.org/comm-central/rev/
> e62f73758e138743a47f2682f721862f09cd89c4/mail/components/compose/content/
> MsgComposeCommands.js#2607

Thank you. That's great information. Currently, editor does not check the DOM tree after modifying DOM tree by itself unless there were some crash bugs. So, basically, we still don't care mutation observer/events in editor.

> For the second point I can understand better what's going on than for the
> first.
> 
> When you start a new e-mail you get a new windows and a new editor inside
> it. We put all sorts of things into the editor in C++ code, like a
> signature, and when done, the C++ code calls NotifyComposeBodyReadyNew()
> from here:
> https://dxr.mozilla.org/comm-central/rev/
> e62f73758e138743a47f2682f721862f09cd89c4/mailnews/compose/src/nsMsgCompose.
> cpp#1666
> 
> That comes into the JS code in MsgComposeCommands.js and inserts a
> paragraph. As you know, TB now defaults into "paragraph mode".
> https://dxr.mozilla.org/comm-central/rev/
> e62f73758e138743a47f2682f721862f09cd89c4/mail/components/compose/content/
> MsgComposeCommands.js#437
> 
> So something triggers the warning when inserting the paragraph via
> InsertElementAtSelection().

Thank you for the investigation. This file is really interesting to me since it actually uses editor's XPCOM APIs. I'm always afraid to breaks something of Thunderbird when we work on improving editor performance. So, this file helps me to understand what's going on in Thunderbird.

Comment 16

[Jorg K (CEST = GMT+2)]

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #15)
> This file is really interesting to me since
> it actually uses editor's XPCOM APIs.
MsgComposeCommands.js is the code that drives the Thunderbird compose window. But there is much more.

In Netscape, all editor code lived in editor/. Once Firefox and Thunderbird separated, editor/ got split into
M-C:
editor/composer
editor/libeditor
and more.

C-C:
editor/ui, also used by the SeaMonkey HTML editor.

Thunderbird/SeaMonkey code is the biggest user of Core::Editor. Further reading in:
https://dxr.mozilla.org/comm-central/source/editor/ui/composer/content/editor.js
and a lot in:
https://dxr.mozilla.org/comm-central/source/editor/ui/dialogs/content,
just picking an example at random:
https://dxr.mozilla.org/comm-central/rev/03203045182581bdc9e8909165c671809aed044a/editor/ui/dialogs/content/EdLinkProps.js#317

Comment 17

[Makoto Kato [:m_kato]]

Comment on attachment 8948164 [details]
Bug 1434969 - EditorDOMPointBase::RewindOffset() shouldn't treat it as error if offset is same as length of the container

https://reviewboard.mozilla.org/r/217740/#review223548

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b5b67bfa069b
EditorDOMPointBase::RewindOffset() shouldn't treat it as error if offset is same as length of the container r=m_kato

Comment 19

[ISHIKAWA, Chiaki]

(In reply to Jorg K (GMT+1) from comment #6)
> This one |'mOffset.value() >= mParent->Length()' (EditorDOMPoint.h, line
> 500)| shows up if you start a new message in TB, that is, open a new compose
> window. Stack:

Thank you for collecting the stack information. I was away on a trip and then was down with a cold-like symptom. 

I may want to figure out if  '!mParent || mOffset.value() <= mParent->Length()' (EditorDOMPoint.h, line 373))
still persists in |make mozmill| after applying the above patch.
The discussion so far suggests  mParent may have been null in the following cases:
> * 350: Main Thread] WARNING: The offset is out of bounds: '!mParent || mOffset.value() <= mParent->Length()', file /NREF-COMM-CENTRAL/objdir-tb3/dist/include/mozilla/EditorDOMPoint.h, line 373 


> 
> _NS_InvokeByIndex() indicates that something is run from JS, in this case
> the JS stack is:

How do you obtain JS stack back trace?
 
> 0 NotifyComposeBodyReadyNew()
> ["chrome://messenger/content/messengercompose/MsgComposeCommands.js":437]
>     this = [object Object]
> 1 NotifyComposeBodyReady()
> ["chrome://messenger/content/messengercompose/MsgComposeCommands.js":352]
>     this = [object Object]
> 2 InitEditor()
> ["chrome://messenger/content/messengercompose/MsgComposeCommands.js":6495]
>     this = [object ChromeWindow]
> 3 observe(aSubject = [xpconnect wrapped nsICommandManager @ 0x193c99a0
> (native @
>  0x20feefc0)], aTopic = "obs_documentCreated", aData =
> "command_status_changed")
>  ["chrome://messenger/content/messengercompose/MsgComposeCommands.js":3004]
>     this = [object Object]
> 

Do you call a function from a debugger?
Which function exactly?
(Oh well I would insert MOZ_ASSERT() as crude measure).

TIA

> MsgComposeCommands.js:437 inserts a paragraph that contains a break:
>       editor.selection.collapse(mailBody, 0);
>       let pElement = editor.createElementWithDefaults("p");
>       pElement.appendChild(editor.createElementWithDefaults("br"));
> 437   editor.insertElementAtSelection(pElement, false);
> 
> Let me know if you want me to do further debugging. Note: I didn't look at
> the Mozmill logs, I just started a local debug build and created a new
> message et voilà.

Comment 20

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/b5b67bfa069b

Comment 21

[Jorg K (CEST = GMT+2)]

(In reply to ISHIKAWA, Chiaki from comment #19)
> How do you obtain JS stack back trace?
You get the debugger to evaluate/execute |DumpJSStack()|. With MS VS you can do that in a QuickWatch window.

Comment 22

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

(In reply to ISHIKAWA, Chiaki from comment #19)
> I may want to figure out if  '!mParent || mOffset.value() <=
> mParent->Length()' (EditorDOMPoint.h, line 373))
> still persists in |make mozmill| after applying the above patch.
> The discussion so far suggests  mParent may have been null in the following
> cases:
> > * 350: Main Thread] WARNING: The offset is out of bounds: '!mParent || mOffset.value() <= mParent->Length()', file /NREF-COMM-CENTRAL/objdir-tb3/dist/include/mozilla/EditorDOMPoint.h, line 373 

Yeah, it's really different bug. It must detect actual bug. Please file a follow up bug with stack trace when you hit it.

Comment 23

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

Comment on attachment 8948164 [details]
Bug 1434969 - EditorDOMPointBase::RewindOffset() shouldn't treat it as error if offset is same as length of the container

Approval Request Comment
[Feature/Bug causing the regression]:
Regression of bug 1419745 (starting from 59).

[User impact if declined]:
User may see odd result when they modify contenteditable or designMode editor since HTMLEditRules::PromoteRange() which is used to prepare of some edit actions may hits this bug.

[Is this code covered by automated tests?]:
No.

[Has the fix been verified in Nightly?]:
Yes.

[Needs manual test from QE? If yes, steps to reproduce]: 
No since we don't have a good STR to reproduce actual symptom.

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

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
This patch just fixes wrong offset range check in a DOM node. Max possible offset of DOM point is length of the container node. However, old code decided offset == container.length is illegal. And adjusting (decrementing) offset is canceled in such case.

[String changes made/needed]:
No.

Comment 24

[Jorg K (CEST = GMT+2)]

OK, this landed this morning on C-C and I'm not looking at the Mozmill debug run which includes this patch:
In this log
https://public-artifacts.taskcluster.net/F9XHhuioSjq-0-cjoVk8uw/0/public/logs/live_backing.log
I see
WARNING: The offset is out of bounds: '!mParent || mOffset.value() <= mParent->Length()', file z:\build\build\src\obj-thunderbird\dist\include\mozilla/EditorDOMPoint.h, line 373
351 times.

I also see
WARNING: '!mOffset.value()', file z:\build\build\src\obj-thunderbird\dist\include\mozilla/EditorDOMPoint.h, line 499
eight times.

I'll file a new bug at least for the message that shows up 351 times. BTW, I can't reproduce it right now by just creating a new compose windows in TB.

Comment 25

[Jorg K (CEST = GMT+2)]

I filed follow-up bug 1436216.

Comment 26

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8948164 [details]
Bug 1434969 - EditorDOMPointBase::RewindOffset() shouldn't treat it as error if offset is same as length of the container

Sounds like we can assume this recent regression may affect Firefox users as well as TB. Let's uplift for beta 9.

Comment 27

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/releases/mozilla-beta/rev/eca8ed852633