1393337 - Get rid of nsIEditRules

Status: RESOLVED FIXED

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

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

nsIEditRules is an abstract class of TextEditRules and HTMLEditRules. However, HTMLEditRules is a subclass of TextEditRules and nsIEditRules is not referred by any other modules. So, we can just get rid of the interface and could make some methods of them non-virtual.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

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

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

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

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: Bug 1393337 - Don't try to remove whitespaces in WSRunObject::ConvertToNBSP() when the text node is changed by mutation observer

WSRunObject::ConvertToNBSP() inserts an NBSP, then, removes following ASCII
whitespaces.  When inserting an NBSP, mutation observer may change the
text node.  In this case, it shouldn't keep working on removing ASCII
whitespaces because it may causes unexpected result.

This patch also renames ConvertToNBSP() and GetAsciiWSBounds() to
InsertNBSPAndRemoveFollowingASCIIWhitespaces() and
GetASCIIWhitespacesBounds() for making their jobs clearer.

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

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

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

Comment 5

[Makoto Kato [:m_kato]]

Comment on attachment 8938327 [details]
Bug 1393337 - Don't try to remove whitespaces in WSRunObject::ConvertToNBSP() when the text node is changed by mutation observer

https://reviewboard.mozilla.org/r/209054/#review215006

Is this bug 1425997?

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Oh, sorry I posted different bug's patch here.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: Bug 1393337 - Get rid of nsIEditRules

nsIEditRules is a super class of only mozilla::TextEditRules and not scriptable.
So, we can get rid of it.

This patch merges RulesInfo with TextRulesInfo and name new class is RulesInfo
for minimizing the code change.

Additionally, adds two methods AsHTMLEditRules() and its const version.
They make existing cast code safer.

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

Comment 8

[Makoto Kato [:m_kato]]

Comment on attachment 8938558 [details]
Bug 1393337 - Get rid of nsIEditRules

https://reviewboard.mozilla.org/r/209196/#review215090

As deCom, it is OK.  But I think that some members (such as mTimer) in TextEditRules should be private. Or, we creates EditRules class and HTMLEditRules shouldn't be inheritance class of TextEditRules because some fuctions is only Text mode such as password.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Comment on attachment 8938558 [details]
Bug 1393337 - Get rid of nsIEditRules

https://reviewboard.mozilla.org/r/209196/#review215090

Yeah, looks like that we should clean them up in some follow up bugs.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4fdf6c12d5f
Get rid of nsIEditRules r=m_kato

Comment 11

[Cristina Coroiu [:ccoroiu]]

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

Comment 12

[Gerry Chang [:gchang]]

Too late for FF58. Mark 58 won't fix.