1357642 - Investigate "WARNING: 'mIsInEditAction', file c:/mozilla/src/editor/libeditor/EditorBase.cpp, line 1896"

Status: RESOLVED FIXED

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

Description

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

When you use IME on debug build, you see a lot of warnings about mIsInEditAction.

> WARNING: 'mIsInEditAction', file c:/mozilla/src/editor/libeditor/EditorBase.cpp, line 1896

This is not a new regression, though, we should investigate this. (could be improve debug build's test performance.)

Comment 1

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

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

Comment 2

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

Attachment: Bug 1357642 TextEditor::UpdateIMEComposition() shouldn't call NotifyEditorObservers(eNotifyEditorObserversOfBefore) by itself

TextEditor::UpdateIMEComposition() uses AutoPlaceHolderBatch class which calls NotifyEditorObservers(eNotifyEditorObserversOfBefore) automatically when it's created.  Therefore, it causes calling NotifyEditorObservers(eNotifyEditorObserversOfBefore) twice from TextEditor::UpdateIMEComposition().

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

Comment 3

[Makoto Kato [:m_kato]]

Comment on attachment 8878958 [details]
Bug 1357642 TextEditor::UpdateIMEComposition() shouldn't call NotifyEditorObservers(eNotifyEditorObserversOfBefore) by itself

https://reviewboard.mozilla.org/r/149820/#review154822

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/89d2cb80a186
TextEditor::UpdateIMEComposition() shouldn't call NotifyEditorObservers(eNotifyEditorObserversOfBefore) by itself r=m_kato

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/89d2cb80a186

Comment 6

[Julien Cristau [:jcristau]]

setting 55 status to wontfix based on the "not a new regression" description in comment 0.

Comment 7

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

(In reply to Julien Cristau [:jcristau] from comment #6)
> setting 55 status to wontfix based on the "not a new regression" description
> in comment 0.

Yeah, this only affects the performance of automated tests on Windows. So, it's not worthwhile to uplift it.