1387317 - Editor instance should be referred/stored as concrete class as far as possible part2

Status: RESOLVED FIXED

(Core :: DOM: Editor, enhancement)

Description

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

+++ This bug was initially created as a clone of Bug #1374207 +++

nsIEditor and other related interfaces will be builtin classes after fixing bug 1060051.  Then, all editor instances are guaranteed that the instances are TextEditor or HTMLEditor.  Then, we can reduce a lot of virtual calls with accessing them via pointer to concrete class.

Comment 1

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

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

Comment 2

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

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

Comment 3

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

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

Comment 4

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

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

Comment 5

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

Attachment: Bug 1387317 - part1: Make AutoSelectionSetterAfterTableEdit use HTMLEditor instead of nsITableEditor

nsITableEditor is now a builtin class.  So, it's implemented only by HTMLEditor.  Therefore, AutoSelectionSetterAfterTableEdit can use HTMLEditor.

Then, nsITableEditor.setSelectionAfterTableEdit() can be removed from nsITableEditor and moved to HTMLEditor as non-virtual method since nobody uses it from JS.

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

Comment 6

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

Attachment: Bug 1387317 - part2: EditorEventListener should stop using interface methods as far as possible

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

Comment 7

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

Attachment: Bug 1387317 - part3: HTMLEditor should use RefPtr<HTMLEditor> rather than nsCOMPtr<nsIHTMLEditor> for kungFuDeathGrip

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

Comment 8

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

Attachment: Bug 1387317 - part4: Make editor flag setters use new AddFlags() or RemoveFlags() if useful

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

Comment 9

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

Attachment: Bug 1387317 - part5: AutoPlaceHolderBatch should take EditorBase instead of nsIEditor

AutoPlaceHolderBatch can take EditorBase class and its inherited class, AutoEditBatch, can be removed if we implement other constructor which doesn't take transaction name.

Additionally, nsIEditor::(Begin|End)PlaceHolderTransaction() are referred only by AutoPlaceHolderBatch.  Therefore, they can be non-public methods and removed from nsIEditor interface.

Note that this patch also repalces "PlaceHolder" with "Placeholder" since it's a word.

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

Comment 10

[Makoto Kato [:m_kato]]

Comment on attachment 8897278 [details]
Bug 1387317 - part1: Make AutoSelectionSetterAfterTableEdit use HTMLEditor instead of nsITableEditor

https://reviewboard.mozilla.org/r/168570/#review173898

Comment 11

[Makoto Kato [:m_kato]]

Comment on attachment 8897279 [details]
Bug 1387317 - part2: EditorEventListener should stop using interface methods as far as possible

https://reviewboard.mozilla.org/r/168572/#review173938

Comment 12

[Makoto Kato [:m_kato]]

Comment on attachment 8897280 [details]
Bug 1387317 - part3: HTMLEditor should use RefPtr<HTMLEditor> rather than nsCOMPtr<nsIHTMLEditor> for kungFuDeathGrip

https://reviewboard.mozilla.org/r/168574/#review173940

Comment 13

[Makoto Kato [:m_kato]]

Comment on attachment 8897281 [details]
Bug 1387317 - part4: Make editor flag setters use new AddFlags() or RemoveFlags() if useful

https://reviewboard.mozilla.org/r/168576/#review173944

Comment 14

[Makoto Kato [:m_kato]]

Comment on attachment 8897282 [details]
Bug 1387317 - part5: AutoPlaceHolderBatch should take EditorBase instead of nsIEditor

https://reviewboard.mozilla.org/r/168578/#review173956

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c88f2794e675
part1: Make AutoSelectionSetterAfterTableEdit use HTMLEditor instead of nsITableEditor r=m_kato
https://hg.mozilla.org/integration/autoland/rev/eda3fcc1e872
part2: EditorEventListener should stop using interface methods as far as possible r=m_kato
https://hg.mozilla.org/integration/autoland/rev/052c275f3166
part3: HTMLEditor should use RefPtr<HTMLEditor> rather than nsCOMPtr<nsIHTMLEditor> for kungFuDeathGrip r=m_kato
https://hg.mozilla.org/integration/autoland/rev/de61ab519b71
part4: Make editor flag setters use new AddFlags() or RemoveFlags() if useful r=m_kato
https://hg.mozilla.org/integration/autoland/rev/b131a336d02f
part5: AutoPlaceHolderBatch should take EditorBase instead of nsIEditor r=m_kato

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c88f2794e675
https://hg.mozilla.org/mozilla-central/rev/eda3fcc1e872
https://hg.mozilla.org/mozilla-central/rev/052c275f3166
https://hg.mozilla.org/mozilla-central/rev/de61ab519b71
https://hg.mozilla.org/mozilla-central/rev/b131a336d02f