1388269 - Spellchecker should use TextEditor rather than nsIEditor

Status: RESOLVED FIXED

(Core :: DOM: Editor, enhancement)

Comment 1

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

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

Comment 2

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

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

Comment 3

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

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

Comment 4

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

Attachment: Bug 1388269 - part1: mozInlineSpellChecker should store editor as strong pointer instead of weak pointer

mozInlineSpellChecker is a cycle collectable class.  Therefore, with including mEditor to the cycle, we can make it nsCOMPtr<nsIEditor> instead of nsWeakPtr.

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

Comment 5

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

Attachment: Bug 1388269 - part2: Make mozInlineSpellChecker store TextEditor instead of nsIEditor

Then, can reduce a lot of unnecessary virtual calls and QI.

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

Comment 6

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

Attachment: Bug 1388269 - part3: spellchecker should use TextEditor instead of nsIEditor

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

Comment 7

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

Attachment: Bug 1388269 - part4: Make mozInlineSpellChecker::GetSpellCheckSelection() return Selection instead of nsISelection

Although, this is not necessary for bug 1388269, we should fix this here since this change is really similar to what bug 1388269 tries to fix and enough simple and safe.

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

Comment 8

[Makoto Kato [:m_kato]]

Comment on attachment 8897273 [details]
Bug 1388269 - part2: Make mozInlineSpellChecker store TextEditor instead of nsIEditor

https://reviewboard.mozilla.org/r/168558/#review173852

::: extensions/spellcheck/src/mozInlineSpellChecker.cpp:1841
(Diff revision 1)
> -
> +  }
> -  rv = selection->GetFocusNode(getter_AddRefs(mCurrentSelectionAnchorNode));
> -  NS_ENSURE_SUCCESS(rv, rv);
>  
> -  selection->GetFocusOffset(&mCurrentSelectionOffset);
> +  mCurrentSelectionAnchorNode = do_QueryInterface(selection->GetFocusNode());
> +  mCurrentSelectionOffset = selection->FocusOffset();

I will file a new bug that mCurrentSelectionAnchorNode changes to nsINode and mCurrentSelectionOffset changes to uint32_t.

Comment 9

[Makoto Kato [:m_kato]]

Comment on attachment 8897274 [details]
Bug 1388269 - part3: spellchecker should use TextEditor instead of nsIEditor

https://reviewboard.mozilla.org/r/168560/#review173860

Comment 10

[Makoto Kato [:m_kato]]

Comment on attachment 8897275 [details]
Bug 1388269 - part4: Make mozInlineSpellChecker::GetSpellCheckSelection() return Selection instead of nsISelection

https://reviewboard.mozilla.org/r/168562/#review173864

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8897272 [details]
Bug 1388269 - part1: mozInlineSpellChecker should store editor as strong pointer instead of weak pointer

https://reviewboard.mozilla.org/r/168556/#review173888

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/353c67a074cd
part1: mozInlineSpellChecker should store editor as strong pointer instead of weak pointer r=smaug
https://hg.mozilla.org/integration/autoland/rev/85709484582e
part2: Make mozInlineSpellChecker store TextEditor instead of nsIEditor r=m_kato
https://hg.mozilla.org/integration/autoland/rev/5d275d296b7e
part3: spellchecker should use TextEditor instead of nsIEditor r=m_kato
https://hg.mozilla.org/integration/autoland/rev/3dfde1e86b56
part4: Make mozInlineSpellChecker::GetSpellCheckSelection() return Selection instead of nsISelection r=m_kato

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/353c67a074cd
https://hg.mozilla.org/mozilla-central/rev/85709484582e
https://hg.mozilla.org/mozilla-central/rev/5d275d296b7e
https://hg.mozilla.org/mozilla-central/rev/3dfde1e86b56