1425390 - Make CompositionTransaction initializes itself with TextComposition

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

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

I'd like to clean up around creating transactions.  However, CompositionTransaction blocks my idea because it takes a lot of local information at creating its instance.  It should be redesigned with TextComposition.

Comment 1

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

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

Comment 2

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

Attachment: Bug 1425390 - part 1: Move some IME related members of EditorBase into TextComposition

EditorBase stores a text node, offset in it and length in it of composition
string directly.  However, this wastes memory space if user never uses IME
or user only sometimes uses IME.  Additionally, storing all data in
TextComposition is better than current design when other classes like
CompositionTransaction wants some information of both EditorBase and
TextComposition.

This patch moves those 3 members from EditorBase to TextComposition.

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

Comment 3

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

Attachment: Bug 1425390 - part 2: Make the constructor of CompositionTransaction take TextComposition to reduce its arguments

Most arguments of the constructor of CompositionTransaction are now stored by
EditorBase::mComposition.  So, making it take TextComposition reduces the
number of its arguments.

Note that this patch doesn't make it retrieve TextComposition with
EditorBase::GetComposition() for guaranteeing that the editor has non-nullptr
mComposition.

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

Comment 4

[Makoto Kato [:m_kato]]

Comment on attachment 8937014 [details]
Bug 1425390 - part 1: Move some IME related members of EditorBase into TextComposition

https://reviewboard.mozilla.org/r/207744/#review213648

Comment 5

[Makoto Kato [:m_kato]]

Comment on attachment 8937015 [details]
Bug 1425390 - part 2: Make the constructor of CompositionTransaction take TextComposition to reduce its arguments

https://reviewboard.mozilla.org/r/207746/#review213652

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d913369aa5ae
part 1: Move some IME related members of EditorBase into TextComposition r=m_kato
https://hg.mozilla.org/integration/autoland/rev/99f9646ec458
part 2: Make the constructor of CompositionTransaction take TextComposition to reduce its arguments r=m_kato

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/d913369aa5ae
https://hg.mozilla.org/mozilla-central/rev/99f9646ec458