1260651 - Rename classes under editor/libeditor and move the namespace from global to mozilla

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

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

First of all, I'd like to rename the classes under editor/libeditor and move the classes from global namespace to mozilla namespace.

I'm thinking that:

nsEditor -> mozilla::AEditor (A prefix is typically used for abstract class)
nsPlaintextEditor -> mozilla::TextEditor ("plaintext" sounds redundant...)
nsHTMLEditor -> mozilla::HTMLEditor

*Txn -> mozilla::*Transaction (I don't like to use abbreviations as far as possible)

ns*EditRules -> mozilla::*EditRules (I don't better idea, though. "Rules" sounds like the class does nothing actually, but it edits the editor content actually...)

ns*EditorUtils -> mozilla::*EditorUtils

Ehsan, do you have any ideas about this?

Comment 1

[(no longer active)]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #0)
> First of all, I'd like to rename the classes under editor/libeditor and move
> the classes from global namespace to mozilla namespace.
> 
> I'm thinking that:
> 
> nsEditor -> mozilla::AEditor (A prefix is typically used for abstract class)

Is it?  The only examples I can think of off the top of my head are nsAString and nsACString, and in my experience nobody understands what that "A" means.  :-)

How about EditorCommon?

> nsPlaintextEditor -> mozilla::TextEditor ("plaintext" sounds redundant...)
> nsHTMLEditor -> mozilla::HTMLEditor
> 
> *Txn -> mozilla::*Transaction (I don't like to use abbreviations as far as
> possible)
> 
> ns*EditRules -> mozilla::*EditRules (I don't better idea, though. "Rules"
> sounds like the class does nothing actually, but it edits the editor content
> actually...)
> 
> ns*EditorUtils -> mozilla::*EditorUtils
> 
> Ehsan, do you have any ideas about this?

The rest looks good.

One fun point to note about the rules classes.  The comments in the beginning of nsTextEditRules.h try to explain the rationale behind these classes, but IIRC those comments are just lies and in reality I don't think there's a clear reason behind this separation any more.  I wouldn't be too sad if someone merges the rules classes into the editor classes...  Up to you.

Comment 2

[Daniel Glazman (:glazou) (NOT MONITORING BMO ANY MORE)]

(In reply to :Ehsan Akhgari (busy) from comment #1)

> How about EditorCommon?

Yes, that's better. Or EditorBase...
Since TB/PB people and myself are almost the only users of that outside
of FF, I'm not sure it really matters as soon it's readable :-)

Comment 3

[Daniel Glazman (:glazou) (NOT MONITORING BMO ANY MORE)]

The name of nsHTMLEditRules was given by Joe Francis because that file
contains what he called "editing rules". What happens when you change
the semantics of a given element, what happens when you create or split
a list, etc. I've always found that name pretty accurate but I have absolutely
no religion about a complete reorg of nsPlaintextEdit* and nsHTMLEdit*

But if we start reorganizing the editor, the big task is making sure
everything is done and prepared for a JS-based extensibility of the
editor. We should be able in the near-term future to replace small or
large bits of the c++ machinery by JS code, and also extend the rules
through JS. Example: typing on CR inside a list item splits the item
but how can I add, in a SIMPLE manner, a new JS-based behaviour when
the user types shift-CR? That is doable today, but "simple" is not
a word I would use for it.

Comment 4

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

Thank you, Ehsan and glazou.

I like "EditorBase" because some classes in our code use "Base" for super classes.

Note that I don't have any plans to change editor's behavior especially for the compatibility with add-ons. I just want to clean up the code and use modern coding rules like other modules.

> Since TB/PB people and myself are almost the only users of that outside of FF

I don't know what you're doing in your business. What will break your product? Changing XPCOM interfaces which are defined in *.h?

Comment 5

[Daniel Glazman (:glazou) (NOT MONITORING BMO ANY MORE)]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #4)

> I don't know what you're doing in your business. What will break your
> product? Changing XPCOM interfaces which are defined in *.h?

I am the author of a few products around editing, all based on Gecko. The
main ones are BlueGriffon ( http://bluegriffon.org ) and BlueGriffon EPUB
Edition ( http://www.bluegriffon-epubedition.com/BGEE.html ). The former is
a wysiwyg cross-platform web editor dealing with all flavors of html
including the most powerful css editor on the market; quite a large success.
The latter is the *only* native Ebook wysiwyg editor conformant to both
EPUB2 and EPUB3 on the whole world-wide market; no proprietary pivot format there.

To build them, I'm applying some rather light patches to mozilla-central [1]
but most of my code is in JS. What would possibly break BlueGriffon is a reorg
of the IDLs. Example: I'm QIing to nsITableEditor when I need to apply changes
to a html table. If that interface goes away, I need to tweak my code. Not an issue
if all changes are well documented, of course.

So all in all, I have no objection to your proposal, on the contrary. It feels
really good to see a new contributor to the editor step in to maintain a bit
that crucial - but old - bit of code. FWIW, I have dissected most existing
wysiwyg editors on the markets, all platforms; Gecko, with its aging core editor,
remains on top of the list and by far! It's the most powerful and the most forward-
thinking of all. I should have myself took care of it, but it was too time-consuming
for me and was harming my revenue stream... Sigh.

[1] https://github.com/therealglazou/bluegriffon/blob/bg2/config/content.patch

Comment 6

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

Thank you very much for the information.

Currently, I don't have any plans to reorganizing XPIDL. So, don't worry about that.

Okay, I *might* reorganize nsI* in C++ header file if it makes the design simpler. But for now, I focus on cleaning up the old style code.

Comment 7

[Mats Palmgren (inactive)]

After you land these name changes I think it would be a good idea to make
a list of all the old/new name pairs (including namespace(s)) so that someone
can make an automated update to the crash signature field in Bugzilla (adding
the new names, keeping the old).

I've seen it happen before that bugs are closed as WFM because there are no
new crashes reported, when the real reason is that the method was renamed.

Comment 8

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Please don't write the patches until my editing bugs are landed, to avoid massive conflicts.  I should land them in the next couple of weeks unless there's a holdup on review.

Comment 9

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

(In reply to Mats Palmgren (:mats) from comment #7)
> After you land these name changes I think it would be a good idea to make
> a list of all the old/new name pairs (including namespace(s)) so that someone
> can make an automated update to the crash signature field in Bugzilla (adding
> the new names, keeping the old).

Good point. Where should I post the list? dev-platforms?

Comment 10

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

(In reply to :Aryeh Gregor (not working until April 10-May 8) from comment #8)
> Please don't write the patches until my editing bugs are landed, to avoid
> massive conflicts.  I should land them in the next couple of weeks unless
> there's a holdup on review.

Although, I've already started to write the patches. But no problem, they must be easy to merge because my patches are almost only doing simple replacement.

But of course, I hope you'll land them as soon as possible. Are you going to go vacation during 4/10 - 5/8? If the landing became after 5/8, I'd be very unhappy though...

Comment 11

[Jorg K (CEST = GMT+2)]

Aryeh will be working from April 10th to May 8th (if I interpret this correctly).

Comment 12

[Mats Palmgren (inactive)]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #9)
> Good point. Where should I post the list? dev-platforms?

Probably just file a bug under bugzilla.mozilla.org/Administration ?
I know we have updated the Crash Signature fields before, without sending bugmail,
but I don't recall who did it.  (it was for signature changes in Socorro)

Comment 13

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

I'm working now for the next few weeks and my first priority is to go through these patch sets and see what the review status is.  If they look like they're stalled more than that, I'm fine with you going ahead and landing and I'll sort out the conflicts, as long as there's a complete list of the name changes.

Or, if someone can work out a git filter-branch command that will magically update my patches, land anytime you want.  (I just tried to get git filter-branch to work for nsRefPtr -> RefPtr, and failed, but probably someone else can figure it out.)

Comment 14

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

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

Comment 15

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

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

Comment 16

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

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

Comment 17

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

Attachment: Bug 1260651 part.1 Rename nsEditorUtils to mozilla::EditorUtils (and their files too)

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

Comment 18

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

Attachment: Bug 1260651 part.2 Rename nsEditorHookUtils to mozilla::EditorHookUtils

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

Comment 19

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

Attachment: Bug 1260651 part.3 Rename DOMPoint to mozilla::EditorDOMPoint because same name class is used in other modules widely

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

Comment 20

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

Attachment: Bug 1260651 part.4 Rename nsTrivialFunctor to mozilla::TrivialFunctor

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

Comment 21

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

Attachment: Bug 1260651 part.5 Rename nsDOMSubtreeIterator to mozilla::DOMSubtreeIterator

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

Comment 22

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

Attachment: Bug 1260651 part.6 Rename nsDOMIterator to mozilla::DOMIterator

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

Comment 23

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

Attachment: Bug 1260651 part.7 Rename nsBoolDomIterFunctor to mozilla::BoolDomIterFunctor

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

Comment 24

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

Attachment: Bug 1260651 part.8 Rename nsAutoUpdateViewBatch to mozilla::AutoUpdateViewBatch

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

Comment 25

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

Attachment: Bug 1260651 part.9 Rename nsAutoTxnsConserveSelection to mozilla::AutoTransactionsConserveSelection

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

Comment 26

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

Attachment: Bug 1260651 part.10 Rename nsAutoRules to mozilla::AutoRules

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

Comment 27

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

Attachment: Bug 1260651 part.11 Rename nsAutoSelectionReset to mozilla::AutoSelectionRestorer

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

Comment 28

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

Attachment: Bug 1260651 part.12 Rename nsAutoEditBatch to mozilla::AutoEditBatch

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

Comment 29

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

Attachment: Bug 1260651 part.13 Rename nsAutoPlaceHolderBatch to mozilla::AutoPlaceHolderBatch

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

Comment 30

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

Attachment: Bug 1260651 part.14 Rename nsTextEditUtils to mozilla::TextEditUtils (and their files too)

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

Comment 31

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

Attachment: Bug 1260651 part.15 Rename nsAutoEditInitRulesTrigger to mozilla::AutoEditInitRulesTrigger

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

Comment 32

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

Attachment: Bug 1260651 part.16 Rename nsHTMLEditUtils to mozilla::HTMLEditUtils (and their files too)

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

Comment 33

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

Attachment: Bug 1260651 part.17 Rename mozilla::dom::ChangeAttributeTxn to mozilla::ChangeAttributeTransaction (and their files too)

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

Comment 34

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

Attachment: Bug 1260651 part.18 Rename mozilla::dom::ChangeStyleTxn to mozilla::ChangeStyleTransaction (and their files too)

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

Comment 35

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

Attachment: Bug 1260651 part.19 Rename mozilla::dom::CreateElementTxn to mozilla::CreateElementTransaction (and their files too)

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

Comment 36

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

Attachment: Bug 1260651 part.20 Rename DeleteNodeTxn to mozilla::DeleteNodeTransaction (and their files too)

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

Comment 37

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

Attachment: Bug 1260651 part.21 Rename DeleteRangeTxn to mozilla::DeleteRangeTransaction (and their files too)

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

Comment 38

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

Attachment: Bug 1260651 part.22 Rename mozilla::dom::DeleteTextTxn to mozilla::DeleteTextTransaction (and their files too)

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

Comment 39

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

Attachment: Bug 1260651 part.23 Rename mozilla::dom::IMETextTxn to mozilla::CompositionTransaction (and their files too)

This patch renames IMETextTxn to CompositionTransaction.  "Composition" is now used in some web standard specs, e.g., CompositionEvent defined by UI Events.

This patch also renames nsEditor::CreateTxnForIMEText() to nsEditor::CreateTxnForComposition().

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

Comment 40

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

Attachment: Bug 1260651 part.24 Rename mozilla::dom::InsertNodeTxn to mozilla::InsertNodeTransaction (and their files too)

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

Comment 41

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

Attachment: Bug 1260651 part.25 Rename mozilla::dom::InsertTextTxn to mozilla::InsertTextTransaction (and their files too)

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

Comment 42

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

Attachment: Bug 1260651 part.26 Rename mozilla::dom::JoinNodeTxn to mozilla::JoinNodeTransaction (and their files too)

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

Comment 43

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

Attachment: Bug 1260651 part.27 Rename PlaceholderTxn to mozilla::PlaceholderTransaction (and their files too)

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

Comment 44

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

Attachment: Bug 1260651 part.28 Rename SetDocTitleTxn to mozilla::SetDocumentTitleTransaction (and their files too)

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

Comment 45

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

Attachment: Bug 1260651 part.29 Rename mozilla::dom::SplitNodeTxn to mozilla::SplitNodeTransaction (and their files too)

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

Comment 46

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

Attachment: Bug 1260651 part.30 Rename EditAggregateTxn to mozilla::EditAggregateTransaction (and their files too)

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

Comment 47

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

Attachment: Bug 1260651 part.31 Rename AddStyleSheetTxn and RemoveStyleSheetTxn to mozilla::AddStyleSheetTransaction and mozilla::RemoveStyleSheetTransaction

This patch also moves ImplCycleCollectionTraverse() and ImplCycleCollectionUnlink() to mozilla namespace for avoiding bustage due to confusion caused by "using namespace" in other unified cpp files.

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

Comment 48

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

Attachment: Bug 1260651 part.32 Rename EditTxn to mozilla::EditTransactionBase

This patch also renames NS_DECL_EDITTXN to NS_DECL_EDIT_TRANSACTION_BASE.

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

Comment 49

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

Attachment: Bug 1260651 part.33 Rename nsEditorController to mozilla::EditorController (and their files too)

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

Comment 50

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

Attachment: Bug 1260651 part.34 Rename editor command classes and their file names

This patch renames editor command classes listed below:

* nsBaseEditorCommand -> mozilla::EditorCommandBase
* nsUndoCommand -> mozilla::UndoCommand
* nsRedoCommand -> mozilla::RedoCommand
* nsClearUndoCommand -> mozilla::ClearUndoCommand
* nsCutCommand -> mozilla::CutCommand
* nsCutOrDeleteCommand -> mozilla::CutOrDeleteCommand
* nsCopyCommand -> mozilla::CopyCommand
* nsCopyOrDeleteCommand -> mozilla::CopyOrDeleteCommand
* nsCopyAndCollapseToEndCommand -> mozilla::CopyAndCollapseToEndCommand
* nsPasteCommand -> mozilla::PasteCommand
* nsPasteTransferableCommand -> mozilla::PasteTransferableCommand
* nsSwitchTextDirectionCommand -> mozilla::SwitchTextDirectionCommand
* nsDeleteCommand -> mozilla::DeleteCommand
* nsSelectAllCommand -> mozilla::SelectAllCommand
* nsSelectionMoveCommands -> mozilla::SelectionMoveCommands
* nsInsertPlaintextCommand -> mozilla::InsertPlaintextCommand
* nsPasteQuotationCommand -> mozilla::PasteQuotationCommand

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

Comment 51

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

Attachment: Bug 1260651 part.35 Move PorpItem and TypeInState from global namespace to mozilla namespace

Moving PropItem and TypeInState into mozilla namespace. I.e., these names are:
PropItem -> mozilla::PropItem
TypeInState -> mozilla::TypeInState

Although, there might be better names for them.  They are too general names.

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

Comment 52

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

Attachment: Bug 1260651 part.36 Rename nsInternetCiter to mozilla::InternetCiter (and their files too)

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

Comment 53

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

Attachment: Bug 1260651 part.37 Rename nsSelectionState to mozilla::SelectionState (and their file names too)

Note that this fixes some new bustage of nsHTMLEditor.

nbsp is conflict with nsWSRunObject.cpp's same name constant.  Therefore, I moved it into nsHTMLEditor and rename it to kNBSP.

And including some missing header files.

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

Comment 54

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

Attachment: Bug 1260651 part.38 Rename nsRangeStore to mozilla::RangeItem because the instances called 'item' in many places

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

Comment 55

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

Attachment: Bug 1260651 part.39 Rename nsRangeUpdater to mozilla::RangeUpdater

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

Comment 56

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

Attachment: Bug 1260651 part.40 Rename nsAutoTrackDOMPoint to mozilla::AutoTrackDOMPoint

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

Comment 57

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

Attachment: Bug 1260651 part.41 Rename mozilla::dom::AutoReplaceContainerSelNotify to mozilla::AutoReplaceContainerSelNotify

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

Comment 58

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

Attachment: Bug 1260651 part.42 Rename nsAutoRemoveContainerSelNotify to mozilla::AutoRemoveContainerSelNotify

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

Comment 59

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

Attachment: Bug 1260651 part.43 Rename nsAutoInsertContainerSelNotify to mozilla::AutoInsertContainerSelNotify

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

Comment 60

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

Attachment: Bug 1260651 part.44 Rename nsAutoMoveNodeSelNotify to mozilla::AutoMoveNodeSelNotify

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

Comment 61

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

Attachment: Bug 1260651 part.45 Rename nsHTMLCSSUtils to mozilla::CSSEditUtils (and also their file names)

This patch also renames nsHTMLEditor::mHTMLCSSUtils to nsHTMLEditor::mCSSEditUtils.

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

Comment 62

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

Attachment: Bug 1260651 part.46 Rename nsHTMLURIRefObject to mozilla::HTMLURIRefObject (and also their file names)

This patch also fixes new bustage of nsHTMLEditRules.cpp and nsWSRunObject.cpp.

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

Comment 63

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

Attachment: Bug 1260651 part.47 Rename nsWSRunObject to mozilla::WSRunObject (and also their file names)

Perhaps, there may be better name like WhitespaceRunObject or something, however, for now keep using the term because I don't understand well what it does.

With this patch, following objects are renamed:

nsWSRunObject -> mozilla::WSRunObject
WSType -> mozilla::WSType
nsWSRunObject::WSFragment -> mozilla::WSRunObject::WSFragment
nsWSRunObject::WSPoint -> mozilla::WSRunObject::WSPoint

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

Comment 64

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

Attachment: Bug 1260651 part.48 Rename nsHTMLEditRules to mozilla::HTMLEditRules (and also their file names)

This patch changes following classes/structs:

nsHTMLEditRules -> mozilla::HTMLEditRules
StyleCache -> mozilla::StyleCache
nsTableCellAndListItemFunctor -> mozilla::TableCellAndListItemFunctor
nsBRNodeFunctor -> mozilla::BRNodeFunctor
nsEmptyEditableFunctor -> mozilla::EmptyEditableFunctor
nsUniqueFunctor -> mozilla::UniqueFunctor

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

Comment 65

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

Attachment: Bug 1260651 part.49 Rename nsTextEditRules to mozilla::TextEditRules (and also their file names)

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

Comment 66

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

Attachment: Bug 1260651 part.50 Rename nsTextRulesInfo to mozilla::TextRulesInfo

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

Comment 67

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

Attachment: Bug 1260651 part.51 Rename nsAutoLockRulesSniffing to mozilla::AutoLockRulesSniffing

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

Comment 68

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

Attachment: Bug 1260651 part.52 Rename nsAutoLockListener to mozilla::AutoLockListener

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

Comment 69

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

Attachment: Bug 1260651 part.53 Rename nsRulesInfo to mozilla::RulesInfo and rename nsEditRules.h to nsIEditRules.h

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

Comment 70

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

Attachment: Bug 1260651 part.54 Rename nsHTMLEditorEventListeners to mozilla::HTMLEditorEventListener (and their file names)

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

Comment 71

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

Attachment: Bug 1260651 part.55 Rename nsEditorEventListeners to mozilla::EditorEventListener (and their file names)

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

Comment 72

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

Attachment: Bug 1260651 part.56 Rename nsHTMLEditor to mozilla::HTMLEditor and related stuff

This patch renames classes/structs as:

nsHTMLEditor -> mozilla::HTMLEditor
nsHTMLEditor::BlobReader -> mozilla::HTMLEditor::BlobReader
SetSelectionAfterTableEdit -> mozilla::AutoSelectionSetterAfterTableEdit

nsHTMLEditor.h -> HTMLEditor.h (exposed as mozilla/editor/HTMLEditor.h)
nsHTMLAbsPosition.cpp -> HTMLAbsPositionEditor.cpp
nsHTMLAnonymousUtils.cpp -> HTMLAnonymousNodeEditor.cpp
nsHTMLDataTransfer.cpp -> HTMLEditorDataTransfer.cpp
nsHTMLEditorStyle.cpp -> HTMLStyleEditor.cpp
nsHTMLInlineTableEditor.cpp -> HTMLInlineTableEditor.cpp
nsHTMLObjectResizer.cpp -> HTMLEditorObjectResizer.cpp
nsTableEditor.cpp -> HTMLTableEditor.cpp

These new file names are clearer names which related to HTMLEditor than old names.

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

Comment 73

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

Attachment: Bug 1260651 part.57 Move classes in nsHTMLObjectResizer.h into mozilla namespace and its file name should be HTMLEditorObjectResizerUtils.h

This patch renames:

ResizerSelectionListener -> mozilla::ResizerSelectionListener
ResizerMouseMotionListener -> mozilla::ResizerMouseMotionListener
DocumentResizeEventListener -> mozilla::DocumentResizeEventListener

And making the header file name HTMLEditorObjectResizerUtils.h because it doesn't define specific class related to its file name and should be clearer that it's related to HTMLEditor.

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

Comment 74

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

Attachment: Bug 1260651 part.58 Rename nsPlaintextEditor to mozilla::TextEditor (and their file names too)

This patch renames nsPlaintextEditor to mozilla::TextEditor.

Additionally, renames TextEditRules::mEditor to TextEditRules::mTextEditor for making its type clearer.

Finally, renaming following files:

nsPlaintextEditor.h -> TextEditor.h (exposed as mozilla/editor/TextEditor.h)
nsPlaintextEditor.cpp -> TextEditor.cpp
nsPlaintextDataTransfer.cpp -> TextEditorDataTransfer.cpp

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

Comment 75

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

Attachment: Bug 1260651 part.59 Rename nsEditor to mozilla::EditorBase (and also their file names)

This patch also renames:

EditorInputEventDispatcher -> mozilla::EditorInputEventDispatcher

And some variable names are renamed from aEditor or mEditor to aEditorBase or mEditorBase for making their types clearer.

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

Comment 76

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

Attachment: Bug 1260651 part.60 editor/libeditor should export some headers which are required by other modules and other modules shouldn't use local include for them

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

Comment 77

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

I hope that even if you have some concern about some trouble of our coding rules or some nits, I'd like to land them because they can be broken with a tiny patch. So, I hope that you'll accept that I'll fix some problems which are not critical in follow up bugs.

Comment 78

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

smaug:

Could you review the patches? Ehsan is sick (according to his account) and he's already agreed the new names in comment 1. So, remaining issue is, who can review them. Even if Ehsan doesn't like some new names, we can rename them easy. So, for now, we should land them as soon as possible. For suppressing bug spams, I don't change the review requests, but I hope you to review them.

Comment 79

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

I think I could review next week.

Comment 80

[Andrew McCreight [:mccr8]]

If this is really just a matter of doing a bunch of renames that Ehsan already agreed to, I could review that, if Olli is okay with that.

Comment 81

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

totally fine. thanks.

Comment 82

[Andrew McCreight [:mccr8]]

Comment on attachment 8765111 [details]
Bug 1260651 part.1 Rename nsEditorUtils to mozilla::EditorUtils (and their files too)

https://reviewboard.mozilla.org/r/48089/#review59460

This looks fine to me. My comments below are just minor gripes that don't require any changes. :)

::: editor/libeditor/PlaceholderTxn.h:10
(Diff revision 1)
>  
>  #ifndef AggregatePlaceholderTxn_h__
>  #define AggregatePlaceholderTxn_h__
>  
>  #include "EditAggregateTxn.h"
> -#include "nsEditorUtils.h"
> +#include "EditorUtils.h"

Ideally, this would be #include "mozilla/EditorUtils.h", but froydnj said you can't really do this without exporting the header, so this is fine.

::: editor/libeditor/nsEditor.cpp:19
(Diff revision 1)
>  #include "CreateElementTxn.h"           // for CreateElementTxn
>  #include "DeleteNodeTxn.h"              // for DeleteNodeTxn
>  #include "DeleteRangeTxn.h"             // for DeleteRangeTxn
>  #include "DeleteTextTxn.h"              // for DeleteTextTxn
>  #include "EditAggregateTxn.h"           // for EditAggregateTxn
> +#include "EditorUtils.h"                // for nsAutoRules, etc

Technically this should be "etc." instead of "etc", but I see the file uses "etc" everywhere so that's okay.

::: editor/libeditor/EditorUtils.h:8
(Diff revision 1)
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  
> -#ifndef nsEditorUtils_h__
> -#define nsEditorUtils_h__
> +#ifndef EditorUtils_h_
> +#define EditorUtils_h_

Per the Mozilla coding style, this should not have any trailing underscores, but this is okay because you aren't making it any worse. :)

::: editor/libeditor/EditorUtils.h:268
(Diff revision 1)
>    }
>  };
>  
> +namespace mozilla {
>  
> -class nsEditorUtils
> +class EditorUtils final

It would be easier to review future patches like this if you split out minor fixups like this into a patch separate from mechanical changes like mass renamings. It also would make it easier to bisect in case of any problems.

Comment 83

[Andrew McCreight [:mccr8]]

I guess I opened issues for each of my comments and I shouldn't have? I'm not sure, I don't use MozReview much. Anyways feel free to ignore them or cancel them as appropriate.

Comment 84

[Andrew McCreight [:mccr8]]

Comment on attachment 8765112 [details]
Bug 1260651 part.2 Rename nsEditorHookUtils to mozilla::EditorHookUtils

https://reviewboard.mozilla.org/r/48091/#review59470

Comment 85

[Andrew McCreight [:mccr8]]

https://reviewboard.mozilla.org/r/48093/#review59472

::: editor/libeditor/EditorUtils.h:249
(Diff revision 1)
>  {
>    nsCOMPtr<nsINode> node;
>    int32_t offset;
>  
> -  DOMPoint() : node(nullptr), offset(-1) {}
> -  DOMPoint(nsINode* aNode, int32_t aOffset)
> +  EditorDOMPoint()
> +    : node(nullptr)

FWIW, this initialization is unnecessary because node is an nsCOMPtr.

::: editor/libeditor/nsHTMLEditRules.h:145
(Diff revision 1)
>                                 bool* aCancel, bool* aHandled);
>    nsresult DidDeleteSelection(Selection* aSelection,
>                                nsIEditor::EDirection aDir,
>                                nsresult aResult);
>    nsresult InsertBRIfNeeded(Selection* aSelection);
> -  ::DOMPoint GetGoodSelPointForNode(nsINode& aNode,
> +  mozilla::EditorDOMPoint GetGoodSelPointForNode(nsINode& aNode,

Seems a little odd that EditorDOMPoint has to be explicitly qualified while Element doesn't, but maybe something weird is going on with Element.

Comment 86

[Andrew McCreight [:mccr8]]

Comment on attachment 8765113 [details]
Bug 1260651 part.3 Rename DOMPoint to mozilla::EditorDOMPoint because same name class is used in other modules widely

https://reviewboard.mozilla.org/r/48093/#review59476

Comment 87

[Andrew McCreight [:mccr8]]

Comment on attachment 8765114 [details]
Bug 1260651 part.4 Rename nsTrivialFunctor to mozilla::TrivialFunctor

https://reviewboard.mozilla.org/r/48095/#review59478

Comment 88

[Andrew McCreight [:mccr8]]

Comment on attachment 8765115 [details]
Bug 1260651 part.5 Rename nsDOMSubtreeIterator to mozilla::DOMSubtreeIterator

https://reviewboard.mozilla.org/r/48097/#review59482

Comment 89

[Andrew McCreight [:mccr8]]

Comment on attachment 8765116 [details]
Bug 1260651 part.6 Rename nsDOMIterator to mozilla::DOMIterator

https://reviewboard.mozilla.org/r/48099/#review59486

Comment 90

[Andrew McCreight [:mccr8]]

Comment on attachment 8765117 [details]
Bug 1260651 part.7 Rename nsBoolDomIterFunctor to mozilla::BoolDomIterFunctor

https://reviewboard.mozilla.org/r/48101/#review59488

Comment 91

[Andrew McCreight [:mccr8]]

Comment on attachment 8765118 [details]
Bug 1260651 part.8 Rename nsAutoUpdateViewBatch to mozilla::AutoUpdateViewBatch

https://reviewboard.mozilla.org/r/48103/#review59490

::: editor/libeditor/EditorUtils.h:178
(Diff revision 1)
> -
> -  explicit nsAutoUpdateViewBatch(nsEditor *ed MOZ_GUARD_OBJECT_NOTIFIER_PARAM) : mEd(ed)
> +  explicit AutoUpdateViewBatch(nsEditor* aEditor
> +                               MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
> +    : mEditor(aEditor)
>    {
>      MOZ_GUARD_OBJECT_NOTIFIER_INIT;
> -    NS_ASSERTION(mEd, "null mEd pointer!");
> +    NS_ASSERTION(mEditor, "null mEd pointer!");

nit: the assertion comment needs to say mEditor instead of mEd.

Comment 92

[Andrew McCreight [:mccr8]]

Comment on attachment 8765119 [details]
Bug 1260651 part.9 Rename nsAutoTxnsConserveSelection to mozilla::AutoTransactionsConserveSelection

https://reviewboard.mozilla.org/r/48105/#review59492

::: editor/libeditor/EditorUtils.h:137
(Diff revision 1)
>  
>  /***************************************************************************
>   * stack based helper class for turning off active selection adjustment
>   * by low level transactions
>   */
> -class MOZ_RAII nsAutoTxnsConserveSelection
> +class MOZ_RAII AutoTransactionsConserveSelection final

Great, now I know what "Txn" stands for. :)

Comment 93

[Andrew McCreight [:mccr8]]

Comment on attachment 8765120 [details]
Bug 1260651 part.10 Rename nsAutoRules to mozilla::AutoRules

https://reviewboard.mozilla.org/r/48107/#review59500

Comment 94

[Andrew McCreight [:mccr8]]

Comment on attachment 8765121 [details]
Bug 1260651 part.11 Rename nsAutoSelectionReset to mozilla::AutoSelectionRestorer

https://reviewboard.mozilla.org/r/48109/#review59508

<p>This rename is outside of the scope of the renames described in comment 0, so you may want to ask Ehsan if he is okay with it. It seems reasonable to me, given that the comment uses the word "restore" but not "reset".</p>

::: editor/libeditor/nsHTMLEditRules.cpp:3057
(Diff revision 1)
>  
>      // remember our new block for postprocessing
>      mNewBlock = theListItem;
>      // put selection in new list item
>      res = aSelection->Collapse(theListItem, 0);
> -    // to prevent selection resetter from overriding us
> +    // Prevent to restore selection.

nit: I think the new comment is ungramattical. Maybe "Don't restore the selection" instead. Here and below in a few places. Or just omit the comments, as they doesn't give any more information than the Abort() call itself.

::: editor/libeditor/nsHTMLEditRules.cpp:4578
(Diff revision 1)
>      *aHandled = true;
>      // Put in a moz-br so that it won't get deleted
>      rv = CreateMozBR(div->AsDOMNode(), 0);
>      NS_ENSURE_SUCCESS(rv, rv);
>      rv = aSelection.Collapse(div, 0);
>      // Don't reset our selection in this case.

nit: Update or delete this comment.

::: editor/libeditor/nsHTMLEditRules.cpp:8555
(Diff revision 1)
>        NS_ENSURE_SUCCESS(res, res);
>        arrayOfNodes.RemoveElementAt(0);
>      }
>      // Put selection in new block
>      res = aSelection.Collapse(positionedDiv, 0);
>      // Prevent selection resetter from overriding us.

nit: Update or delete this comment.

Comment 95

[Andrew McCreight [:mccr8]]

Comment on attachment 8765122 [details]
Bug 1260651 part.12 Rename nsAutoEditBatch to mozilla::AutoEditBatch

https://reviewboard.mozilla.org/r/48111/#review59524

Comment 96

[Andrew McCreight [:mccr8]]

Comment on attachment 8765123 [details]
Bug 1260651 part.13 Rename nsAutoPlaceHolderBatch to mozilla::AutoPlaceHolderBatch

https://reviewboard.mozilla.org/r/48113/#review59526

::: editor/libeditor/EditorUtils.h:45
(Diff revision 1)
> -    nsCOMPtr<nsIEditor> mEd;
> +  nsCOMPtr<nsIEditor> mEditor;
> -    MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
> +  MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
> +
> -  public:
> +public:
> -    nsAutoPlaceHolderBatch(nsIEditor *aEd, nsIAtom *atom MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
> -      : mEd(do_QueryInterface(aEd))
> +  AutoPlaceHolderBatch(nsIEditor* aEditor,
> +                       nsIAtom* aAtom

This QI is odd. It looks like it was never actually needed, going back even to the initial landing. Oh well.

Comment 97

[Andrew McCreight [:mccr8]]

Comment on attachment 8765124 [details]
Bug 1260651 part.14 Rename nsTextEditUtils to mozilla::TextEditUtils (and their files too)

https://reviewboard.mozilla.org/r/48115/#review59528

Comment 98

[Andrew McCreight [:mccr8]]

Comment on attachment 8765125 [details]
Bug 1260651 part.15 Rename nsAutoEditInitRulesTrigger to mozilla::AutoEditInitRulesTrigger

https://reviewboard.mozilla.org/r/48117/#review59532

Comment 99

[Andrew McCreight [:mccr8]]

Comment on attachment 8765126 [details]
Bug 1260651 part.16 Rename nsHTMLEditUtils to mozilla::HTMLEditUtils (and their files too)

https://reviewboard.mozilla.org/r/48119/#review59538

I reviewed the raw diff to HTMLEditUtils.h because it was broken in ReviewBoard (there's an existing bug on that)

::: editor/libeditor/HTMLEditUtils.cpp:118
(Diff revision 1)
>  }
>  
>  bool
> -nsHTMLEditUtils::IsHeader(nsIDOMNode* aNode)
> +HTMLEditUtils::IsHeader(nsIDOMNode* aNode)
>  {
> +  MOZ_ASSERT(aNode);

Why did you change the assert? It seems like before it was checking the result of the QI, while now it isn't.

Comment 100

[Andrew McCreight [:mccr8]]

Comment on attachment 8765127 [details]
Bug 1260651 part.17 Rename mozilla::dom::ChangeAttributeTxn to mozilla::ChangeAttributeTransaction (and their files too)

https://reviewboard.mozilla.org/r/48121/#review59548

::: editor/libeditor/ChangeAttributeTransaction.h:56
(Diff revision 1)
> -  nsCOMPtr<Element> mElement;
> +  nsCOMPtr<dom::Element> mElement;
>  
> -  /** The attribute to change */
> +  // The attribute to change
>    nsCOMPtr<nsIAtom> mAttribute;
>  
> -  /** The value to set the attribute to (ignored if mRemoveAttribute==true) */
> +  // The value to set the attribute to (ignored if mRemoveAttribute==true) 

nit: trailing space.

Comment 101

[Andrew McCreight [:mccr8]]

Comment on attachment 8765128 [details]
Bug 1260651 part.18 Rename mozilla::dom::ChangeStyleTxn to mozilla::ChangeStyleTransaction (and their files too)

https://reviewboard.mozilla.org/r/48123/#review59550

Comment 102

[Andrew McCreight [:mccr8]]

Comment on attachment 8765129 [details]
Bug 1260651 part.19 Rename mozilla::dom::CreateElementTxn to mozilla::CreateElementTransaction (and their files too)

https://reviewboard.mozilla.org/r/48125/#review59552

Comment 103

[Andrew McCreight [:mccr8]]

Comment on attachment 8765130 [details]
Bug 1260651 part.20 Rename DeleteNodeTxn to mozilla::DeleteNodeTransaction (and their files too)

https://reviewboard.mozilla.org/r/48127/#review59554

::: editor/libeditor/DeleteNodeTransaction.cpp:16
(Diff revision 1)
>  #include "nsAString.h"
>  
> -using namespace mozilla;
> +namespace mozilla {
>  
> -DeleteNodeTxn::DeleteNodeTxn()
> -  : EditTxn(), mNode(), mParent(), mRefNode(), mRangeUpdater(nullptr)
> +DeleteNodeTransaction::DeleteNodeTransaction()
> +  : EditTxn()

Do you actually need to explicitly call the argument-less parent ctor?

Comment 104

[Andrew McCreight [:mccr8]]

Comment on attachment 8765131 [details]
Bug 1260651 part.21 Rename DeleteRangeTxn to mozilla::DeleteRangeTransaction (and their files too)

https://reviewboard.mozilla.org/r/48129/#review59558

::: editor/libeditor/DeleteRangeTransaction.cpp:29
(Diff revision 1)
> -using namespace mozilla::dom;
> +
> +using namespace dom;
>  
>  // note that aEditor is not refcounted
> -DeleteRangeTxn::DeleteRangeTxn()
> -  : EditAggregateTxn(),
> +DeleteRangeTransaction::DeleteRangeTransaction()
> +  : EditAggregateTxn()

I think you don't need this explicit ctor call.

Comment 105

[Andrew McCreight [:mccr8]]

Comment on attachment 8765132 [details]
Bug 1260651 part.22 Rename mozilla::dom::DeleteTextTxn to mozilla::DeleteTextTransaction (and their files too)

https://reviewboard.mozilla.org/r/48131/#review59560

Comment 106

[Andrew McCreight [:mccr8]]

Comment on attachment 8765133 [details]
Bug 1260651 part.23 Rename mozilla::dom::IMETextTxn to mozilla::CompositionTransaction (and their files too)

https://reviewboard.mozilla.org/r/48133/#review59568

::: editor/libeditor/CompositionTransaction.h:28
(Diff revision 1)
> -
>  class Text;
> +} // namespace dom
>  
>  /**
> -  * A transaction that inserts text into a content node.
> + * A transaction that inserts or modifies composition string into a content

The grammar seems a off in this comment. strings instead of string maybe?

::: editor/libeditor/PlaceholderTxn.cpp:23
(Diff revision 1)
> -                                    mAbsorb(true),
> -                                    mForwarding(nullptr),
> -                                    mIMETextTxn(nullptr),
> -                                    mCommitted(false),
> -                                    mStartSel(nullptr),
> -                                    mEndSel(),
> +  : EditAggregateTxn()
> +  , mAbsorb(true)
> +  , mForwarding(nullptr)
> +  , mCompositionTransaction(nullptr)
> +  , mCommitted(false)
> +  , mStartSel(nullptr)

mStartSel doesn't need to be initialized. Also you probably don't need to explicitly call parent ctor.

Comment 107

[Andrew McCreight [:mccr8]]

Comment on attachment 8765134 [details]
Bug 1260651 part.24 Rename mozilla::dom::InsertNodeTxn to mozilla::InsertNodeTransaction (and their files too)

https://reviewboard.mozilla.org/r/48135/#review59570

Comment 108

[Andrew McCreight [:mccr8]]

Comment on attachment 8765135 [details]
Bug 1260651 part.25 Rename mozilla::dom::InsertTextTxn to mozilla::InsertTextTransaction (and their files too)

https://reviewboard.mozilla.org/r/48137/#review59572

Comment 109

[Andrew McCreight [:mccr8]]

I've tried to mass-clear the review requests for Ehsan, but I get the error message "Unable to update reviewers as the review request has pending changes (the patch author has a draft)" in review board for some reason.

Comment 110

[Andrew McCreight [:mccr8]]

Comment on attachment 8765136 [details]
Bug 1260651 part.26 Rename mozilla::dom::JoinNodeTxn to mozilla::JoinNodeTransaction (and their files too)

https://reviewboard.mozilla.org/r/48139/#review59574

Comment 111

[Andrew McCreight [:mccr8]]

Comment on attachment 8765137 [details]
Bug 1260651 part.27 Rename PlaceholderTxn to mozilla::PlaceholderTransaction (and their files too)

https://reviewboard.mozilla.org/r/48141/#review59576

::: editor/libeditor/PlaceholderTransaction.h:83
(Diff revision 1)
> -  // This is so that UndoTransaction() and RedoTransaction() can restore the
> -  // selection properly.
> -  nsAutoPtr<nsSelectionState> mStartSel; // use a pointer because this is constructed before we exist
> +  // Selection at the start of the transaction is stored, as is the selection
> +  // at the end.  This is so that UndoTransaction() and RedoTransaction() can
> +  // restore the selection properly.
> +
> +  // Use a pointer because this is constructed before we exist.
> +  nsAutoPtr<nsSelectionState> mStartSel; 

nit: trailing whitespace

Comment 112

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

https://reviewboard.mozilla.org/r/48089/#review59460

> Ideally, this would be #include "mozilla/EditorUtils.h", but froydnj said you can't really do this without exporting the header, so this is fine.

Yes. I don't want to expose unnecessary headers...

> Technically this should be "etc." instead of "etc", but I see the file uses "etc" everywhere so that's okay.

Thank you. I got it. When I use "etc", I'll use "etc." instead.

> Per the Mozilla coding style, this should not have any trailing underscores, but this is okay because you aren't making it any worse. :)

Oh, you're right. I've not known the new rule. I'll change it since I don't like to violate the rules by myself.

> It would be easier to review future patches like this if you split out minor fixups like this into a patch separate from mechanical changes like mass renamings. It also would make it easier to bisect in case of any problems.

Yeah. But I probably only add "final" keywords (or maybe also add "MOZ_STACK_CLASS") only when it doesn't cause bustage, I include such changes. Other changes are renaming and cleaning up due to too wrong line or ununified comment style. Sorry for the messy patches.

Comment 113

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

https://reviewboard.mozilla.org/r/48093/#review59472

> Seems a little odd that EditorDOMPoint has to be explicitly qualified while Element doesn't, but maybe something weird is going on with Element.

Because Element is redefined here: https://dxr.mozilla.org/mozilla-central/rev/95ffbc4ff63584631c408e8d9912961fcf68bb09/editor/libeditor/nsTextEditRules.h#42-45
I think that my changes list up which classes which are not in global namespace are used a lot. Anyway, |mozilla::| will be removed in other patches.

Comment 114

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

https://reviewboard.mozilla.org/r/48119/#review59538

> Why did you change the assert? It seems like before it was checking the result of the QI, while now it isn't.

Because I checked the all callers and they never pass nullptr for this method. So, my MOZ_ASSERT() must make sense. However, as you said, we should check after QI too (I guess, it never happens though). I'll fix it before landing.

Comment 115

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

https://reviewboard.mozilla.org/r/48109/#review59508

> nit: I think the new comment is ungramattical. Maybe "Don't restore the selection" instead. Here and below in a few places. Or just omit the comments, as they doesn't give any more information than the Abort() call itself.

All comments are replaced with as you suggested. Thank you.

Comment 116

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

https://reviewboard.mozilla.org/r/48103/#review59490

> nit: the assertion comment needs to say mEditor instead of mEd.

Fixed.

Comment 117

[Andrew McCreight [:mccr8]]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #114)
> Because I checked the all callers and they never pass nullptr for this
> method. So, my MOZ_ASSERT() must make sense. However, as you said, we should
> check after QI too (I guess, it never happens though). I'll fix it before
> landing.

I did notice this same pattern in a bunch of other places, and this was the only one that was checking after the QI. It may just be an accident that this one is different. I don't know enough about the code to tell. (And a QI of null will just turn into null so you are still checking that the initial argument is non-null.)

Comment 118

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

https://reviewboard.mozilla.org/r/48121/#review59548

> nit: trailing space.

fixed.

Comment 119

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

https://reviewboard.mozilla.org/r/48127/#review59554

> Do you actually need to explicitly call the argument-less parent ctor?

Indeed, and I realized that mEditor isn't initialized. So, I'll remove the call of argument-less parent ctor and add mEditor(nullptr).

Comment 120

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

https://reviewboard.mozilla.org/r/48129/#review59558

> I think you don't need this explicit ctor call.

Sure.

Comment 121

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

https://reviewboard.mozilla.org/r/48133/#review59568

> The grammar seems a off in this comment. strings instead of string maybe?

Um, no. "composition string" is a term of IME.

I updated the comment to:

/**
 * CompositionTransaction stores all edit for a composition, i.e.,
 * from compositionstart event to compositionend event.  E.g., inserting a
 * composition string, modifying the composition string or its IME selection
 * ranges and commit or cancel the composition.
 */

> mStartSel doesn't need to be initialized. Also you probably don't need to explicitly call parent ctor.

Sure.

Comment 122

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

https://reviewboard.mozilla.org/r/48141/#review59576

> nit: trailing whitespace

Fixed.

Comment 123

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

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

Comment 124

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

Comment on attachment 8765111 [details]
Bug 1260651 part.1 Rename nsEditorUtils to mozilla::EditorUtils (and their files too)

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

Comment 125

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

Comment on attachment 8765112 [details]
Bug 1260651 part.2 Rename nsEditorHookUtils to mozilla::EditorHookUtils

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

Comment 126

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

Comment on attachment 8765113 [details]
Bug 1260651 part.3 Rename DOMPoint to mozilla::EditorDOMPoint because same name class is used in other modules widely

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

Comment 127

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

Comment on attachment 8765114 [details]
Bug 1260651 part.4 Rename nsTrivialFunctor to mozilla::TrivialFunctor

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

Comment 128

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

Comment on attachment 8765115 [details]
Bug 1260651 part.5 Rename nsDOMSubtreeIterator to mozilla::DOMSubtreeIterator

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

Comment 129

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

Comment on attachment 8765116 [details]
Bug 1260651 part.6 Rename nsDOMIterator to mozilla::DOMIterator

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

Comment 130

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

Comment on attachment 8765117 [details]
Bug 1260651 part.7 Rename nsBoolDomIterFunctor to mozilla::BoolDomIterFunctor

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

Comment 131

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

Comment on attachment 8765118 [details]
Bug 1260651 part.8 Rename nsAutoUpdateViewBatch to mozilla::AutoUpdateViewBatch

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

Comment 132

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

Comment on attachment 8765119 [details]
Bug 1260651 part.9 Rename nsAutoTxnsConserveSelection to mozilla::AutoTransactionsConserveSelection

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

Comment 133

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

Comment on attachment 8765120 [details]
Bug 1260651 part.10 Rename nsAutoRules to mozilla::AutoRules

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

Comment 134

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

Comment on attachment 8765121 [details]
Bug 1260651 part.11 Rename nsAutoSelectionReset to mozilla::AutoSelectionRestorer

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

Comment 135

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

Comment on attachment 8765122 [details]
Bug 1260651 part.12 Rename nsAutoEditBatch to mozilla::AutoEditBatch

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

Comment 136

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

Comment on attachment 8765123 [details]
Bug 1260651 part.13 Rename nsAutoPlaceHolderBatch to mozilla::AutoPlaceHolderBatch

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

Comment 137

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

Comment on attachment 8765124 [details]
Bug 1260651 part.14 Rename nsTextEditUtils to mozilla::TextEditUtils (and their files too)

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

Comment 138

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

Comment on attachment 8765125 [details]
Bug 1260651 part.15 Rename nsAutoEditInitRulesTrigger to mozilla::AutoEditInitRulesTrigger

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

Comment 139

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

Comment on attachment 8765126 [details]
Bug 1260651 part.16 Rename nsHTMLEditUtils to mozilla::HTMLEditUtils (and their files too)

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

Comment 140

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

Comment on attachment 8765127 [details]
Bug 1260651 part.17 Rename mozilla::dom::ChangeAttributeTxn to mozilla::ChangeAttributeTransaction (and their files too)

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

Comment 141

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

Comment on attachment 8765128 [details]
Bug 1260651 part.18 Rename mozilla::dom::ChangeStyleTxn to mozilla::ChangeStyleTransaction (and their files too)

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

Comment 142

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

Comment on attachment 8765129 [details]
Bug 1260651 part.19 Rename mozilla::dom::CreateElementTxn to mozilla::CreateElementTransaction (and their files too)

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

Comment 143

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

Comment on attachment 8765130 [details]
Bug 1260651 part.20 Rename DeleteNodeTxn to mozilla::DeleteNodeTransaction (and their files too)

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

Comment 144

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

Comment on attachment 8765131 [details]
Bug 1260651 part.21 Rename DeleteRangeTxn to mozilla::DeleteRangeTransaction (and their files too)

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

Comment 145

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

Comment on attachment 8765132 [details]
Bug 1260651 part.22 Rename mozilla::dom::DeleteTextTxn to mozilla::DeleteTextTransaction (and their files too)

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

Comment 146

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

Comment on attachment 8765133 [details]
Bug 1260651 part.23 Rename mozilla::dom::IMETextTxn to mozilla::CompositionTransaction (and their files too)

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

Comment 147

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

Comment on attachment 8765134 [details]
Bug 1260651 part.24 Rename mozilla::dom::InsertNodeTxn to mozilla::InsertNodeTransaction (and their files too)

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

Comment 148

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

Comment on attachment 8765135 [details]
Bug 1260651 part.25 Rename mozilla::dom::InsertTextTxn to mozilla::InsertTextTransaction (and their files too)

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

Comment 149

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

Comment on attachment 8765136 [details]
Bug 1260651 part.26 Rename mozilla::dom::JoinNodeTxn to mozilla::JoinNodeTransaction (and their files too)

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

Comment 150

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

Comment on attachment 8765137 [details]
Bug 1260651 part.27 Rename PlaceholderTxn to mozilla::PlaceholderTransaction (and their files too)

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

Comment 151

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

Comment on attachment 8765138 [details]
Bug 1260651 part.28 Rename SetDocTitleTxn to mozilla::SetDocumentTitleTransaction (and their files too)

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

Comment 152

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

Comment on attachment 8765139 [details]
Bug 1260651 part.29 Rename mozilla::dom::SplitNodeTxn to mozilla::SplitNodeTransaction (and their files too)

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

Comment 153

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

Comment on attachment 8765140 [details]
Bug 1260651 part.30 Rename EditAggregateTxn to mozilla::EditAggregateTransaction (and their files too)

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

Comment 154

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

Comment on attachment 8765141 [details]
Bug 1260651 part.31 Rename AddStyleSheetTxn and RemoveStyleSheetTxn to mozilla::AddStyleSheetTransaction and mozilla::RemoveStyleSheetTransaction

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

Comment 155

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

Comment on attachment 8765142 [details]
Bug 1260651 part.32 Rename EditTxn to mozilla::EditTransactionBase

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

Comment 156

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

Comment on attachment 8765143 [details]
Bug 1260651 part.33 Rename nsEditorController to mozilla::EditorController (and their files too)

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

Comment 157

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

Comment on attachment 8765144 [details]
Bug 1260651 part.34 Rename editor command classes and their file names

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

Comment 158

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

Comment on attachment 8765145 [details]
Bug 1260651 part.35 Move PorpItem and TypeInState from global namespace to mozilla namespace

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

Comment 159

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

Comment on attachment 8765146 [details]
Bug 1260651 part.36 Rename nsInternetCiter to mozilla::InternetCiter (and their files too)

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

Comment 160

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

Comment on attachment 8765147 [details]
Bug 1260651 part.37 Rename nsSelectionState to mozilla::SelectionState (and their file names too)

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

Comment 161

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

Comment on attachment 8765148 [details]
Bug 1260651 part.38 Rename nsRangeStore to mozilla::RangeItem because the instances called 'item' in many places

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

Comment 162

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

Comment on attachment 8765149 [details]
Bug 1260651 part.39 Rename nsRangeUpdater to mozilla::RangeUpdater

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

Comment 163

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

Comment on attachment 8765150 [details]
Bug 1260651 part.40 Rename nsAutoTrackDOMPoint to mozilla::AutoTrackDOMPoint

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

Comment 164

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

Comment on attachment 8765151 [details]
Bug 1260651 part.41 Rename mozilla::dom::AutoReplaceContainerSelNotify to mozilla::AutoReplaceContainerSelNotify

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

Comment 165

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

Comment on attachment 8765152 [details]
Bug 1260651 part.42 Rename nsAutoRemoveContainerSelNotify to mozilla::AutoRemoveContainerSelNotify

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

Comment 166

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

Comment on attachment 8765153 [details]
Bug 1260651 part.43 Rename nsAutoInsertContainerSelNotify to mozilla::AutoInsertContainerSelNotify

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

Comment 167

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

Comment on attachment 8765154 [details]
Bug 1260651 part.44 Rename nsAutoMoveNodeSelNotify to mozilla::AutoMoveNodeSelNotify

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

Comment 168

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

Comment on attachment 8765155 [details]
Bug 1260651 part.45 Rename nsHTMLCSSUtils to mozilla::CSSEditUtils (and also their file names)

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

Comment 169

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

Comment on attachment 8765156 [details]
Bug 1260651 part.46 Rename nsHTMLURIRefObject to mozilla::HTMLURIRefObject (and also their file names)

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

Comment 170

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

Comment on attachment 8765157 [details]
Bug 1260651 part.47 Rename nsWSRunObject to mozilla::WSRunObject (and also their file names)

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

Comment 171

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

Comment on attachment 8765158 [details]
Bug 1260651 part.48 Rename nsHTMLEditRules to mozilla::HTMLEditRules (and also their file names)

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

Comment 172

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

Comment on attachment 8765159 [details]
Bug 1260651 part.49 Rename nsTextEditRules to mozilla::TextEditRules (and also their file names)

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

Comment 173

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

Comment on attachment 8765160 [details]
Bug 1260651 part.50 Rename nsTextRulesInfo to mozilla::TextRulesInfo

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

Comment 174

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

Comment on attachment 8765161 [details]
Bug 1260651 part.51 Rename nsAutoLockRulesSniffing to mozilla::AutoLockRulesSniffing

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

Comment 175

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

Comment on attachment 8765162 [details]
Bug 1260651 part.52 Rename nsAutoLockListener to mozilla::AutoLockListener

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

Comment 176

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

Comment on attachment 8765163 [details]
Bug 1260651 part.53 Rename nsRulesInfo to mozilla::RulesInfo and rename nsEditRules.h to nsIEditRules.h

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

Comment 177

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

Comment on attachment 8765164 [details]
Bug 1260651 part.54 Rename nsHTMLEditorEventListeners to mozilla::HTMLEditorEventListener (and their file names)

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

Comment 178

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

Comment on attachment 8765165 [details]
Bug 1260651 part.55 Rename nsEditorEventListeners to mozilla::EditorEventListener (and their file names)

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

Comment 179

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

Comment on attachment 8765166 [details]
Bug 1260651 part.56 Rename nsHTMLEditor to mozilla::HTMLEditor and related stuff

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

Comment 180

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

Comment on attachment 8765167 [details]
Bug 1260651 part.57 Move classes in nsHTMLObjectResizer.h into mozilla namespace and its file name should be HTMLEditorObjectResizerUtils.h

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

Comment 181

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

Comment on attachment 8765168 [details]
Bug 1260651 part.58 Rename nsPlaintextEditor to mozilla::TextEditor (and their file names too)

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

Comment 182

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

Comment on attachment 8765169 [details]
Bug 1260651 part.59 Rename nsEditor to mozilla::EditorBase (and also their file names)

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

Comment 183

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

Comment on attachment 8765170 [details]
Bug 1260651 part.60 editor/libeditor should export some headers which are required by other modules and other modules shouldn't use local include for them

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

Comment 184

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

(In reply to Andrew McCreight [:mccr8] from comment #109)
> I've tried to mass-clear the review requests for Ehsan, but I get the error
> message "Unable to update reviewers as the review request has pending
> changes (the patch author has a draft)" in review board for some reason.

Perhaps, now, it must be available since request to ehsan is canceled. But I think that I can take them as regressions of this bug if Ehsan doesn't like some of new names. (I believe that we should land them as far as possible for unblocking other developers including you.)

Comment 185

[Andrew McCreight [:mccr8]]

> Sorry for the messy patches.

It isn't a huge deal. I can just skim patches faster if they have exactly one kind of change in them, rather than having to keep an eye out for other kinds of changes, too.

Comment 186

[Andrew McCreight [:mccr8]]

Comment on attachment 8765138 [details]
Bug 1260651 part.28 Rename SetDocTitleTxn to mozilla::SetDocumentTitleTransaction (and their files too)

https://reviewboard.mozilla.org/r/60598/#review59754

Comment 187

[Andrew McCreight [:mccr8]]

Comment on attachment 8765139 [details]
Bug 1260651 part.29 Rename mozilla::dom::SplitNodeTxn to mozilla::SplitNodeTransaction (and their files too)

https://reviewboard.mozilla.org/r/60600/#review59756

Comment 188

[Andrew McCreight [:mccr8]]

Comment on attachment 8765140 [details]
Bug 1260651 part.30 Rename EditAggregateTxn to mozilla::EditAggregateTransaction (and their files too)

https://reviewboard.mozilla.org/r/60602/#review59762

Comment 189

[Andrew McCreight [:mccr8]]

Comment on attachment 8765141 [details]
Bug 1260651 part.31 Rename AddStyleSheetTxn and RemoveStyleSheetTxn to mozilla::AddStyleSheetTransaction and mozilla::RemoveStyleSheetTransaction

https://reviewboard.mozilla.org/r/60604/#review59794

Comment 190

[Andrew McCreight [:mccr8]]

Comment on attachment 8765142 [details]
Bug 1260651 part.32 Rename EditTxn to mozilla::EditTransactionBase

https://reviewboard.mozilla.org/r/60606/#review59800

::: editor/libeditor/JoinNodeTransaction.h:9
(Diff revision 2)
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  #ifndef JoinNodeTransaction_h
>  #define JoinNodeTransaction_h
>  
> -#include "EditTxn.h"                    // for EditTxn, NS_DECL_EDITTXN
> +#include "EditTransactionBase.h"        // for EditTransactionBase, etc

micronit: should be "etc."

::: editor/libeditor/PlaceholderTransaction.cpp:129
(Diff revision 2)
>  
>    nsCOMPtr<nsPIEditorTransaction> pTxn = do_QueryInterface(aTransaction);
>    NS_ENSURE_TRUE(pTxn, NS_OK); // it's foreign so just bail!
>  
> -  EditTxn *editTxn = (EditTxn*)aTransaction;  //XXX: hack, not safe!  need nsIEditTransaction!
> +  // XXX: hack, not safe!  need nsIEditTransaction!
> +  EditTransactionBase* editTransactionBase = (EditTransactionBase*)aTransaction;  

nit: trailing whitespace

::: editor/libeditor/PlaceholderTransaction.cpp:131
(Diff revision 2)
>    NS_ENSURE_TRUE(pTxn, NS_OK); // it's foreign so just bail!
>  
> -  EditTxn *editTxn = (EditTxn*)aTransaction;  //XXX: hack, not safe!  need nsIEditTransaction!
> +  // XXX: hack, not safe!  need nsIEditTransaction!
> +  EditTransactionBase* editTransactionBase = (EditTransactionBase*)aTransaction;  
>    // determine if this incoming txn is a placeholder txn
> -  nsCOMPtr<nsIAbsorbingTransaction> plcTxn = do_QueryObject(editTxn);
> +  nsCOMPtr<nsIAbsorbingTransaction> absorvingTransaction =

nit: This should be "absorbingTransaction".

::: editor/libeditor/PlaceholderTransaction.cpp:134
(Diff revision 2)
> +  EditTransactionBase* editTransactionBase = (EditTransactionBase*)aTransaction;  
>    // determine if this incoming txn is a placeholder txn
> -  nsCOMPtr<nsIAbsorbingTransaction> plcTxn = do_QueryObject(editTxn);
> +  nsCOMPtr<nsIAbsorbingTransaction> absorvingTransaction =
> +    do_QueryObject(editTransactionBase);
>  
> -  // we are absorbing all txn's if mAbsorb is lit.
> +  // We are absorbing all transaction's if mAbsorb is lit.

nit: Might as well drop the apostrophe and make this "transactions" while you are here.

Comment 191

[Andrew McCreight [:mccr8]]

Comment on attachment 8765143 [details]
Bug 1260651 part.33 Rename nsEditorController to mozilla::EditorController (and their files too)

https://reviewboard.mozilla.org/r/60608/#review59808

::: editor/libeditor/moz.build:16
(Diff revision 2)
>  
>  MOCHITEST_CHROME_MANIFESTS += ['tests/chrome.ini']
>  
>  BROWSER_CHROME_MANIFESTS += ['tests/browser.ini']
>  
> +EXPORTS.mozilla.editor += [

In my opinion, this should just be in mozilla and not mozilla/editor. I think we try to avoid extra nested namespaces when possible, and "Editor" is in the class name already, so I think it is clear where this is from.

::: editor/libeditor/EditorController.h:6
(Diff revision 2)
>  /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> -#ifndef nsEditorController_h__
> +#ifndef EditorController_h

The include guard should match the include path. In other words, if it is going to be #include "mozilla/EditorController.h" then the include guard should be mozilla_EditorController_h. (from the Mozilla coding style guide: "Include guards are named by determining the fully-qualified include path, then substituting _ for / and . and - in it.")

Comment 192

[Andrew McCreight [:mccr8]]

Comment on attachment 8765144 [details]
Bug 1260651 part.34 Rename editor command classes and their file names

https://reviewboard.mozilla.org/r/60610/#review59822

Comment 193

[Andrew McCreight [:mccr8]]

Comment on attachment 8765145 [details]
Bug 1260651 part.35 Move PorpItem and TypeInState from global namespace to mozilla namespace

https://reviewboard.mozilla.org/r/60612/#review59826

::: editor/libeditor/TypeInState.cpp:391
(Diff revision 2)
> - *    PropItem: helper struct for TypeInState
> + * mozilla::PropItem: helper struct for mozilla::TypeInState
>   *******************************************************************/
>  
> -PropItem::PropItem() :
> - tag(nullptr)
> -,attr()
> +PropItem::PropItem()
> +  : tag(nullptr)
> +  , attr()

I think you don't need to explicitly initialize attr and value, because they are nsString.

Comment 194

[Andrew McCreight [:mccr8]]

Comment on attachment 8765146 [details]
Bug 1260651 part.36 Rename nsInternetCiter to mozilla::InternetCiter (and their files too)

https://reviewboard.mozilla.org/r/60614/#review59830

Comment 195

[Andrew McCreight [:mccr8]]

Comment on attachment 8765147 [details]
Bug 1260651 part.37 Rename nsSelectionState to mozilla::SelectionState (and their file names too)

https://reviewboard.mozilla.org/r/60616/#review59832

for SelectionState.h: In the declaration of SelectionState::SaveSelection, you didn't move the * to the left for aSel.

::: editor/libeditor/nsEditor.h:9
(Diff revision 2)
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  #ifndef __editor_h__
>  #define __editor_h__
>  
> +#include "SelectionState.h"             // for nsRangeUpdater, etc

nit: etc.

Comment 196

[Andrew McCreight [:mccr8]]

Comment on attachment 8765148 [details]
Bug 1260651 part.38 Rename nsRangeStore to mozilla::RangeItem because the instances called 'item' in many places

https://reviewboard.mozilla.org/r/60618/#review59848

Comment 197

[Andrew McCreight [:mccr8]]

Comment on attachment 8765149 [details]
Bug 1260651 part.39 Rename nsRangeUpdater to mozilla::RangeUpdater

https://reviewboard.mozilla.org/r/60620/#review59852

Comment 198

[Andrew McCreight [:mccr8]]

Comment on attachment 8765150 [details]
Bug 1260651 part.40 Rename nsAutoTrackDOMPoint to mozilla::AutoTrackDOMPoint

https://reviewboard.mozilla.org/r/60622/#review59854

Comment 199

[Andrew McCreight [:mccr8]]

Comment on attachment 8765151 [details]
Bug 1260651 part.41 Rename mozilla::dom::AutoReplaceContainerSelNotify to mozilla::AutoReplaceContainerSelNotify

https://reviewboard.mozilla.org/r/60624/#review59880

Comment 200

[Andrew McCreight [:mccr8]]

Comment on attachment 8765152 [details]
Bug 1260651 part.42 Rename nsAutoRemoveContainerSelNotify to mozilla::AutoRemoveContainerSelNotify

https://reviewboard.mozilla.org/r/60626/#review59882

Comment 201

[Andrew McCreight [:mccr8]]

Comment on attachment 8765153 [details]
Bug 1260651 part.43 Rename nsAutoInsertContainerSelNotify to mozilla::AutoInsertContainerSelNotify

https://reviewboard.mozilla.org/r/60628/#review59884

Comment 202

[Andrew McCreight [:mccr8]]

Comment on attachment 8765154 [details]
Bug 1260651 part.44 Rename nsAutoMoveNodeSelNotify to mozilla::AutoMoveNodeSelNotify

https://reviewboard.mozilla.org/r/60630/#review59886

Comment 203

[Andrew McCreight [:mccr8]]

Comment on attachment 8765155 [details]
Bug 1260651 part.45 Rename nsHTMLCSSUtils to mozilla::CSSEditUtils (and also their file names)

https://reviewboard.mozilla.org/r/60632/#review59892

Comment 204

[Andrew McCreight [:mccr8]]

Comment on attachment 8765156 [details]
Bug 1260651 part.46 Rename nsHTMLURIRefObject to mozilla::HTMLURIRefObject (and also their file names)

https://reviewboard.mozilla.org/r/60634/#review59902

Comment 205

[Andrew McCreight [:mccr8]]

Comment on attachment 8765157 [details]
Bug 1260651 part.47 Rename nsWSRunObject to mozilla::WSRunObject (and also their file names)

https://reviewboard.mozilla.org/r/60636/#review59908

Comment 206

[Andrew McCreight [:mccr8]]

Comment on attachment 8765158 [details]
Bug 1260651 part.48 Rename nsHTMLEditRules to mozilla::HTMLEditRules (and also their file names)

https://reviewboard.mozilla.org/r/60638/#review59910

Comment 207

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

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

Comment 208

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

https://reviewboard.mozilla.org/r/60606/#review59800

> micronit: should be "etc."

I'll post part.61 for fixing this issue all over editor/libeditor.

> nit: This should be "absorbingTransaction".

Oops!

Comment 209

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

https://reviewboard.mozilla.org/r/60608/#review59808

> In my opinion, this should just be in mozilla and not mozilla/editor. I think we try to avoid extra nested namespaces when possible, and "Editor" is in the class name already, so I think it is clear where this is from.

Okay, I moved them in new patches.

> The include guard should match the include path. In other words, if it is going to be #include "mozilla/EditorController.h" then the include guard should be mozilla_EditorController_h. (from the Mozilla coding style guide: "Include guards are named by determining the fully-qualified include path, then substituting _ for / and . and - in it.")

Yeah, I checked all changesets again. That's just my mistake.

Comment 210

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

Attachment: Bug 1260651 part.61 Replace "etc" with "etc."

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

Comment 211

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

Comment on attachment 8765142 [details]
Bug 1260651 part.32 Rename EditTxn to mozilla::EditTransactionBase

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

Comment 212

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

Comment on attachment 8765143 [details]
Bug 1260651 part.33 Rename nsEditorController to mozilla::EditorController (and their files too)

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

Comment 213

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

Comment on attachment 8765144 [details]
Bug 1260651 part.34 Rename editor command classes and their file names

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

Comment 214

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

Comment on attachment 8765145 [details]
Bug 1260651 part.35 Move PorpItem and TypeInState from global namespace to mozilla namespace

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

Comment 215

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

Comment on attachment 8765146 [details]
Bug 1260651 part.36 Rename nsInternetCiter to mozilla::InternetCiter (and their files too)

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

Comment 216

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

Comment on attachment 8765147 [details]
Bug 1260651 part.37 Rename nsSelectionState to mozilla::SelectionState (and their file names too)

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

Comment 217

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

Comment on attachment 8765148 [details]
Bug 1260651 part.38 Rename nsRangeStore to mozilla::RangeItem because the instances called 'item' in many places

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

Comment 218

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

Comment on attachment 8765149 [details]
Bug 1260651 part.39 Rename nsRangeUpdater to mozilla::RangeUpdater

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

Comment 219

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

Comment on attachment 8765150 [details]
Bug 1260651 part.40 Rename nsAutoTrackDOMPoint to mozilla::AutoTrackDOMPoint

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

Comment 220

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

Comment on attachment 8765151 [details]
Bug 1260651 part.41 Rename mozilla::dom::AutoReplaceContainerSelNotify to mozilla::AutoReplaceContainerSelNotify

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

Comment 221

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

Comment on attachment 8765152 [details]
Bug 1260651 part.42 Rename nsAutoRemoveContainerSelNotify to mozilla::AutoRemoveContainerSelNotify

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

Comment 222

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

Comment on attachment 8765153 [details]
Bug 1260651 part.43 Rename nsAutoInsertContainerSelNotify to mozilla::AutoInsertContainerSelNotify

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

Comment 223

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

Comment on attachment 8765154 [details]
Bug 1260651 part.44 Rename nsAutoMoveNodeSelNotify to mozilla::AutoMoveNodeSelNotify

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

Comment 224

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

Comment on attachment 8765155 [details]
Bug 1260651 part.45 Rename nsHTMLCSSUtils to mozilla::CSSEditUtils (and also their file names)

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

Comment 225

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

Comment on attachment 8765156 [details]
Bug 1260651 part.46 Rename nsHTMLURIRefObject to mozilla::HTMLURIRefObject (and also their file names)

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

Comment 226

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

Comment on attachment 8765157 [details]
Bug 1260651 part.47 Rename nsWSRunObject to mozilla::WSRunObject (and also their file names)

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

Comment 227

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

Comment on attachment 8765158 [details]
Bug 1260651 part.48 Rename nsHTMLEditRules to mozilla::HTMLEditRules (and also their file names)

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

Comment 228

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

Comment on attachment 8765159 [details]
Bug 1260651 part.49 Rename nsTextEditRules to mozilla::TextEditRules (and also their file names)

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

Comment 229

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

Comment on attachment 8765160 [details]
Bug 1260651 part.50 Rename nsTextRulesInfo to mozilla::TextRulesInfo

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

Comment 230

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

Comment on attachment 8765161 [details]
Bug 1260651 part.51 Rename nsAutoLockRulesSniffing to mozilla::AutoLockRulesSniffing

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

Comment 231

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

Comment on attachment 8765162 [details]
Bug 1260651 part.52 Rename nsAutoLockListener to mozilla::AutoLockListener

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

Comment 232

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

Comment on attachment 8765163 [details]
Bug 1260651 part.53 Rename nsRulesInfo to mozilla::RulesInfo and rename nsEditRules.h to nsIEditRules.h

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

Comment 233

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

Comment on attachment 8765164 [details]
Bug 1260651 part.54 Rename nsHTMLEditorEventListeners to mozilla::HTMLEditorEventListener (and their file names)

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

Comment 234

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

Comment on attachment 8765165 [details]
Bug 1260651 part.55 Rename nsEditorEventListeners to mozilla::EditorEventListener (and their file names)

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

Comment 235

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

Comment on attachment 8765166 [details]
Bug 1260651 part.56 Rename nsHTMLEditor to mozilla::HTMLEditor and related stuff

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

Comment 236

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

Comment on attachment 8765167 [details]
Bug 1260651 part.57 Move classes in nsHTMLObjectResizer.h into mozilla namespace and its file name should be HTMLEditorObjectResizerUtils.h

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

Comment 237

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

Comment on attachment 8765168 [details]
Bug 1260651 part.58 Rename nsPlaintextEditor to mozilla::TextEditor (and their file names too)

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

Comment 238

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

Comment on attachment 8765169 [details]
Bug 1260651 part.59 Rename nsEditor to mozilla::EditorBase (and also their file names)

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

Comment 239

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

Comment on attachment 8765170 [details]
Bug 1260651 part.60 editor/libeditor should export some headers which are required by other modules and other modules shouldn't use local include for them

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

Comment 240

[Andrew McCreight [:mccr8]]

Comment on attachment 8765159 [details]
Bug 1260651 part.49 Rename nsTextEditRules to mozilla::TextEditRules (and also their file names)

https://reviewboard.mozilla.org/r/60640/#review60062

::: editor/libeditor/TextEditRules.cpp:1336
(Diff revision 3)
>  
>    mPasswordIMEText.Assign(*aIMEString);
>  }
>  
> -NS_IMETHODIMP nsTextEditRules::Notify(nsITimer *)
> +NS_IMETHODIMP
> +TextEditRules::Notify(nsITimer *aTimer)

nit: * to the left

::: editor/libeditor/TextEditRules.cpp:1396
(Diff revision 3)
>    for (i=0; i < aLength; i++)
>      aOutString->Append(passwordChar);
>  }
>  
> -
> -///////////////////////////////////////////////////////////////////////////
> +/**
> + * CreateMozBR() puts a BR node with moz attribute at {aNode, aOffset}.

I don't know if you want to bother fixing this, but the comment mentions variables, aNode and aOffset, that aren't actually arguments to the method.

::: layout/xul/tree/nsTreeBodyFrame.cpp
(Diff revision 3)
>  #include "mozilla/dom/TreeBoxObject.h"
>  #include "nsRenderingContext.h"
>  #include "nsIScriptableRegion.h"
>  #include <algorithm>
>  #include "ScrollbarActivity.h"
> -#include "../../editor/libeditor/nsTextEditRules.h"

Hah, that was terrible.

Comment 241

[Andrew McCreight [:mccr8]]

Comment on attachment 8765160 [details]
Bug 1260651 part.50 Rename nsTextRulesInfo to mozilla::TextRulesInfo

https://reviewboard.mozilla.org/r/60642/#review60068

Comment 242

[Andrew McCreight [:mccr8]]

Comment on attachment 8765161 [details]
Bug 1260651 part.51 Rename nsAutoLockRulesSniffing to mozilla::AutoLockRulesSniffing

https://reviewboard.mozilla.org/r/60644/#review60072

Comment 243

[Andrew McCreight [:mccr8]]

Comment on attachment 8765162 [details]
Bug 1260651 part.52 Rename nsAutoLockListener to mozilla::AutoLockListener

https://reviewboard.mozilla.org/r/60646/#review60074

Comment 244

[Andrew McCreight [:mccr8]]

Comment on attachment 8765163 [details]
Bug 1260651 part.53 Rename nsRulesInfo to mozilla::RulesInfo and rename nsEditRules.h to nsIEditRules.h

https://reviewboard.mozilla.org/r/60648/#review60076

Comment 245

[Andrew McCreight [:mccr8]]

Comment on attachment 8765164 [details]
Bug 1260651 part.54 Rename nsHTMLEditorEventListeners to mozilla::HTMLEditorEventListener (and their file names)

https://reviewboard.mozilla.org/r/60650/#review60080

Comment 246

[Andrew McCreight [:mccr8]]

Comment on attachment 8765165 [details]
Bug 1260651 part.55 Rename nsEditorEventListeners to mozilla::EditorEventListener (and their file names)

https://reviewboard.mozilla.org/r/60652/#review60086

Comment 247

[Andrew McCreight [:mccr8]]

Comment on attachment 8765166 [details]
Bug 1260651 part.56 Rename nsHTMLEditor to mozilla::HTMLEditor and related stuff

https://reviewboard.mozilla.org/r/60654/#review60090

::: editor/libeditor/HTMLAbsPositionEditor.cpp:190
(Diff revision 3)
>    return mRules->DidDoAction(selection, &ruleInfo, res);
>  }
>  
>  NS_IMETHODIMP
> -nsHTMLEditor::GetElementZIndex(nsIDOMElement * aElement,
> -                               int32_t * aZindex)
> +HTMLEditor::GetElementZIndex(nsIDOMElement* aElement,
> +                               int32_t* aZindex)

nit: indentation is wrong

::: editor/libeditor/HTMLAnonymousNodeEditor.cpp:136
(Diff revision 3)
>  // child of aParentNode. If aIsCreatedHidden is true, the class
>  // "hidden" is added to the created element. If aAnonClass is not
>  // the empty string, it becomes the value of the attribute "_moz_anonclass"
>  nsresult
> -nsHTMLEditor::CreateAnonymousElement(const nsAString & aTag, nsIDOMNode *  aParentNode,
> -                                     const nsAString & aAnonClass, bool aIsCreatedHidden,
> +HTMLEditor::CreateAnonymousElement(const nsAString& aTag,
> +                                   nsIDOMNode*  aParentNode,

nit: extra space between * and aParentNode.

::: editor/libeditor/HTMLAnonymousNodeEditor.cpp:416
(Diff revision 3)
>  
>  // Resizing and Absolute Positioning need to know everything about the
>  // containing box of the element: position, size, margins, borders
>  nsresult
> -nsHTMLEditor::GetPositionAndDimensions(nsIDOMElement * aElement,
> -                                       int32_t & aX, int32_t & aY,
> +HTMLEditor::GetPositionAndDimensions(nsIDOMElement* aElement,
> +                                       int32_t& aX,

nit: Indentation is off for all of these arguments.

::: editor/libeditor/HTMLEditor.h:934
(Diff revision 3)
>  
> -  // Returns the offset of an element's frame to its absolute containing block.
> -  nsresult GetElementOrigin(nsIDOMElement * aElement, int32_t & aX, int32_t & aY);
> -  nsresult GetPositionAndDimensions(nsIDOMElement * aElement,
> -                                    int32_t & aX, int32_t & aY,
> -                                    int32_t & aW, int32_t & aH,
> +  /**
> +   * Returns the offset of an element's frame to its absolute containing block.
> +   */
> +  nsresult GetElementOrigin(nsIDOMElement* aElement,
> +                            int32_t & aX, int32_t & aY);

nit: space before the & in aX and aY.

::: editor/libeditor/HTMLEditor.cpp:4803
(Diff revision 3)
>    return NS_OK;
>  }
>  
>  nsresult
> -nsHTMLEditor::GetElementOrigin(nsIDOMElement * aElement, int32_t & aX, int32_t & aY)
> +HTMLEditor::GetElementOrigin(nsIDOMElement* aElement,
> +                             int32_t & aX,

nit: extra space in aX and aY lines

::: editor/libeditor/HTMLTableEditor.cpp:6
(Diff revision 3)
>  /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +#include "mozilla/HTMLEditor.h"

This should go after <stdio.h> per the Mozilla coding style.

::: editor/libeditor/HTMLTableEditor.cpp:2707
(Diff revision 3)
>  
>  NS_IMETHODIMP
> -nsHTMLEditor::GetCellDataAt(nsIDOMElement* aTable, int32_t aRowIndex,
> -                            int32_t aColIndex, nsIDOMElement **aCell,
> -                            int32_t* aStartRowIndex, int32_t* aStartColIndex,
> -                            int32_t* aRowSpan, int32_t* aColSpan,
> +HTMLEditor::GetCellDataAt(nsIDOMElement* aTable,
> +                          int32_t aRowIndex,
> +                          int32_t aColIndex,
> +                          nsIDOMElement **aCell,

spacing is wrong

Comment 248

[Andrew McCreight [:mccr8]]

Comment on attachment 8765167 [details]
Bug 1260651 part.57 Move classes in nsHTMLObjectResizer.h into mozilla namespace and its file name should be HTMLEditorObjectResizerUtils.h

https://reviewboard.mozilla.org/r/60656/#review60128

Comment 249

[Andrew McCreight [:mccr8]]

Comment on attachment 8765168 [details]
Bug 1260651 part.58 Rename nsPlaintextEditor to mozilla::TextEditor (and their file names too)

https://reviewboard.mozilla.org/r/60658/#review60132

::: editor/libeditor/TextEditor.h:185
(Diff revision 3)
>                                    uint32_t aFlags,
>                                    const nsACString& aCharset,
>                                    nsIDocumentEncoder** encoder);
>  
> -  // key event helpers
> -  NS_IMETHOD CreateBR(nsIDOMNode *aNode, int32_t aOffset,
> +  NS_IMETHOD CreateBR(nsIDOMNode* aNode, int32_t aOffset,
> +                      nsCOMPtr<nsIDOMNode> *outBRNode,

nit: * should be to the left.

Comment 250

[Andrew McCreight [:mccr8]]

Comment on attachment 8765169 [details]
Bug 1260651 part.59 Rename nsEditor to mozilla::EditorBase (and also their file names)

https://reviewboard.mozilla.org/r/60660/#review60144

Comment 251

[Andrew McCreight [:mccr8]]

Comment on attachment 8765170 [details]
Bug 1260651 part.60 editor/libeditor should export some headers which are required by other modules and other modules shouldn't use local include for them

https://reviewboard.mozilla.org/r/60662/#review60146

Nice.

Comment 252

[Andrew McCreight [:mccr8]]

Comment on attachment 8769056 [details]
Bug 1260651 part.61 Replace "etc" with "etc."

https://reviewboard.mozilla.org/r/63014/#review60148

Thanks. I looked this up on Wikipedia, and apparently "etc." is used in US English, and "etc" is used in UK English. I guess that explains why it is the way it is! Feel free to change it or not as you prefer.

Comment 253

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

https://reviewboard.mozilla.org/r/60640/#review60062

> nit: * to the left

Good catch!

> I don't know if you want to bother fixing this, but the comment mentions variables, aNode and aOffset, that aren't actually arguments to the method.

Modified.

Comment 254

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

https://reviewboard.mozilla.org/r/60654/#review60090

> nit: indentation is wrong

Good catch!

> spacing is wrong

Thank you for your help to clean up the messy editor code!

Comment 255

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

https://reviewboard.mozilla.org/r/60658/#review60132

> nit: * should be to the left.

Thank you very much!!! I'll land them today.

Comment 256

[Andrew McCreight [:mccr8]]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #254)
> Thank you for your help to clean up the messy editor code!

Thanks for fixing it up!

Comment 257

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

Comment on attachment 8765111 [details]
Bug 1260651 part.1 Rename nsEditorUtils to mozilla::EditorUtils (and their files too)

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

Comment 258

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

Comment on attachment 8765112 [details]
Bug 1260651 part.2 Rename nsEditorHookUtils to mozilla::EditorHookUtils

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

Comment 259

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

Comment on attachment 8765113 [details]
Bug 1260651 part.3 Rename DOMPoint to mozilla::EditorDOMPoint because same name class is used in other modules widely

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

Comment 260

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

Comment on attachment 8765114 [details]
Bug 1260651 part.4 Rename nsTrivialFunctor to mozilla::TrivialFunctor

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

Comment 261

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

Comment on attachment 8765115 [details]
Bug 1260651 part.5 Rename nsDOMSubtreeIterator to mozilla::DOMSubtreeIterator

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

Comment 262

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

Comment on attachment 8765116 [details]
Bug 1260651 part.6 Rename nsDOMIterator to mozilla::DOMIterator

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

Comment 263

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

Comment on attachment 8765117 [details]
Bug 1260651 part.7 Rename nsBoolDomIterFunctor to mozilla::BoolDomIterFunctor

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

Comment 264

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

Comment on attachment 8765118 [details]
Bug 1260651 part.8 Rename nsAutoUpdateViewBatch to mozilla::AutoUpdateViewBatch

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

Comment 265

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

Comment on attachment 8765119 [details]
Bug 1260651 part.9 Rename nsAutoTxnsConserveSelection to mozilla::AutoTransactionsConserveSelection

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

Comment 266

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

Comment on attachment 8765120 [details]
Bug 1260651 part.10 Rename nsAutoRules to mozilla::AutoRules

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

Comment 267

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

Comment on attachment 8765121 [details]
Bug 1260651 part.11 Rename nsAutoSelectionReset to mozilla::AutoSelectionRestorer

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

Comment 268

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

Comment on attachment 8765122 [details]
Bug 1260651 part.12 Rename nsAutoEditBatch to mozilla::AutoEditBatch

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

Comment 269

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

Comment on attachment 8765123 [details]
Bug 1260651 part.13 Rename nsAutoPlaceHolderBatch to mozilla::AutoPlaceHolderBatch

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

Comment 270

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

Comment on attachment 8765124 [details]
Bug 1260651 part.14 Rename nsTextEditUtils to mozilla::TextEditUtils (and their files too)

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

Comment 271

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

Comment on attachment 8765125 [details]
Bug 1260651 part.15 Rename nsAutoEditInitRulesTrigger to mozilla::AutoEditInitRulesTrigger

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

Comment 272

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

Comment on attachment 8765126 [details]
Bug 1260651 part.16 Rename nsHTMLEditUtils to mozilla::HTMLEditUtils (and their files too)

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

Comment 273

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

Comment on attachment 8765127 [details]
Bug 1260651 part.17 Rename mozilla::dom::ChangeAttributeTxn to mozilla::ChangeAttributeTransaction (and their files too)

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

Comment 274

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

Comment on attachment 8765128 [details]
Bug 1260651 part.18 Rename mozilla::dom::ChangeStyleTxn to mozilla::ChangeStyleTransaction (and their files too)

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

Comment 275

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

Comment on attachment 8765129 [details]
Bug 1260651 part.19 Rename mozilla::dom::CreateElementTxn to mozilla::CreateElementTransaction (and their files too)

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

Comment 276

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

Comment on attachment 8765130 [details]
Bug 1260651 part.20 Rename DeleteNodeTxn to mozilla::DeleteNodeTransaction (and their files too)

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

Comment 277

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

Comment on attachment 8765131 [details]
Bug 1260651 part.21 Rename DeleteRangeTxn to mozilla::DeleteRangeTransaction (and their files too)

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

Comment 278

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

Comment on attachment 8765132 [details]
Bug 1260651 part.22 Rename mozilla::dom::DeleteTextTxn to mozilla::DeleteTextTransaction (and their files too)

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

Comment 279

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

Comment on attachment 8765133 [details]
Bug 1260651 part.23 Rename mozilla::dom::IMETextTxn to mozilla::CompositionTransaction (and their files too)

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

Comment 280

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

Comment on attachment 8765134 [details]
Bug 1260651 part.24 Rename mozilla::dom::InsertNodeTxn to mozilla::InsertNodeTransaction (and their files too)

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

Comment 281

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

Comment on attachment 8765135 [details]
Bug 1260651 part.25 Rename mozilla::dom::InsertTextTxn to mozilla::InsertTextTransaction (and their files too)

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

Comment 282

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

Comment on attachment 8765136 [details]
Bug 1260651 part.26 Rename mozilla::dom::JoinNodeTxn to mozilla::JoinNodeTransaction (and their files too)

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

Comment 283

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

Comment on attachment 8765137 [details]
Bug 1260651 part.27 Rename PlaceholderTxn to mozilla::PlaceholderTransaction (and their files too)

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

Comment 284

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

Comment on attachment 8765138 [details]
Bug 1260651 part.28 Rename SetDocTitleTxn to mozilla::SetDocumentTitleTransaction (and their files too)

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

Comment 285

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

Comment on attachment 8765139 [details]
Bug 1260651 part.29 Rename mozilla::dom::SplitNodeTxn to mozilla::SplitNodeTransaction (and their files too)

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

Comment 286

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

Comment on attachment 8765140 [details]
Bug 1260651 part.30 Rename EditAggregateTxn to mozilla::EditAggregateTransaction (and their files too)

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

Comment 287

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

Comment on attachment 8765141 [details]
Bug 1260651 part.31 Rename AddStyleSheetTxn and RemoveStyleSheetTxn to mozilla::AddStyleSheetTransaction and mozilla::RemoveStyleSheetTransaction

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

Comment 288

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

Comment on attachment 8765142 [details]
Bug 1260651 part.32 Rename EditTxn to mozilla::EditTransactionBase

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

Comment 289

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

Comment on attachment 8765143 [details]
Bug 1260651 part.33 Rename nsEditorController to mozilla::EditorController (and their files too)

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

Comment 290

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

Comment on attachment 8765144 [details]
Bug 1260651 part.34 Rename editor command classes and their file names

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

Comment 291

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

Comment on attachment 8765145 [details]
Bug 1260651 part.35 Move PorpItem and TypeInState from global namespace to mozilla namespace

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

Comment 292

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

Comment on attachment 8765146 [details]
Bug 1260651 part.36 Rename nsInternetCiter to mozilla::InternetCiter (and their files too)

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

Comment 293

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

Comment on attachment 8765147 [details]
Bug 1260651 part.37 Rename nsSelectionState to mozilla::SelectionState (and their file names too)

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

Comment 294

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

Comment on attachment 8765148 [details]
Bug 1260651 part.38 Rename nsRangeStore to mozilla::RangeItem because the instances called 'item' in many places

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

Comment 295

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

Comment on attachment 8765149 [details]
Bug 1260651 part.39 Rename nsRangeUpdater to mozilla::RangeUpdater

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

Comment 296

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

Comment on attachment 8765150 [details]
Bug 1260651 part.40 Rename nsAutoTrackDOMPoint to mozilla::AutoTrackDOMPoint

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

Comment 297

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

Comment on attachment 8765151 [details]
Bug 1260651 part.41 Rename mozilla::dom::AutoReplaceContainerSelNotify to mozilla::AutoReplaceContainerSelNotify

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

Comment 298

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

Comment on attachment 8765152 [details]
Bug 1260651 part.42 Rename nsAutoRemoveContainerSelNotify to mozilla::AutoRemoveContainerSelNotify

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

Comment 299

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

Comment on attachment 8765153 [details]
Bug 1260651 part.43 Rename nsAutoInsertContainerSelNotify to mozilla::AutoInsertContainerSelNotify

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

Comment 300

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

Comment on attachment 8765154 [details]
Bug 1260651 part.44 Rename nsAutoMoveNodeSelNotify to mozilla::AutoMoveNodeSelNotify

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

Comment 301

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

Comment on attachment 8765155 [details]
Bug 1260651 part.45 Rename nsHTMLCSSUtils to mozilla::CSSEditUtils (and also their file names)

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

Comment 302

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

Comment on attachment 8765156 [details]
Bug 1260651 part.46 Rename nsHTMLURIRefObject to mozilla::HTMLURIRefObject (and also their file names)

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

Comment 303

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

Comment on attachment 8765157 [details]
Bug 1260651 part.47 Rename nsWSRunObject to mozilla::WSRunObject (and also their file names)

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

Comment 304

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

Comment on attachment 8765158 [details]
Bug 1260651 part.48 Rename nsHTMLEditRules to mozilla::HTMLEditRules (and also their file names)

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

Comment 305

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

Comment on attachment 8765159 [details]
Bug 1260651 part.49 Rename nsTextEditRules to mozilla::TextEditRules (and also their file names)

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

Comment 306

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

Comment on attachment 8765160 [details]
Bug 1260651 part.50 Rename nsTextRulesInfo to mozilla::TextRulesInfo

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