1482019 - Move implementation of nsIHTMLEditor::GetSelectedElement() to HTMLEditor::GetSelectedElement()

Status: RESOLVED FIXED

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

Description

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

There is non-virtual overload for HTMLEditor::GetSelectedElement(), however, it calls nsIHTMLEditor::GetSelectedElement() which is of course a virtual method.

Comment 1

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

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

Comment 2

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

Attachment: Bug 1482019 - part 0: Add automated tests for nsIHTMLEditor.getSelectedElement()

The HTMLEditor::GetSelectedElement() is ugly.  Probably, it's checked only with
expected simple cases since the result does not make sense in some cases.

For example, when Selection is collapsed, it returns an element only when
"href" is specified with the argument.  When Selection selects only one
element node (including its children), it quickly returns the node, however,
in the slow path, it returns second element node if first element node matches
with the argument.  Or returns first element ndoe if it does not match with
the argument.

For preventing regressions, new test is designed to keep current odd behavior.

Comment 3

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

Attachment: Bug 1482019 - part 1: Create non-virtual method HTMLEditor::GetSelectedNode() for implementing nsIHTMLEditor::GetSelectedElement()

For making each diff compact, this bug needs some patches.

First of all, this patch moves implementation of
nsIHTMLEditor::GetSelectedElement() to new non-virtual method
HTMLEditor::GetSelectedNode().

Comment 4

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

Attachment: Bug 1482019 - part 2: Make HTMLEditor::GetSelectedNode() take nsAtom* for element name

Using nsAtom makes the method faster and simpler, although the caller needs
to atomize lower-cased string.

Comment 5

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

Attachment: Bug 1482019 - part 3: Minimize some scope of auto varaiables in HTMLEditor::GetSelectedNode()

Some variables in HTMLEditor::GetSelectedNode() are declared very large block
they are not used and/or referred.  So, we can get rid of some variables or
move smaller block.

Comment 6

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

Attachment: Bug 1482019 - part 4: Reduce the indent level of the last block in HTMLEditor::GetSelectedNode()

The first check of the last block of HTMLEditor::GetSelectedNode() can use
early-return style.  Additionally, |currange| is not necessary since Selection
is not changed since the method retrieved first range.  So, we can get rid of
|currange| and the |nullptr| case of it.

Comment 7

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

Attachment: Bug 1482019 - part 5: Make HTMLEditor::GetSelectedNode() never return non-element node and change its name to GetSelectedElement()

If HTMLEditor::GetSelectedNode() is called with nullptr for aTagName,
the first block may return non-element node.  In such case, we should return
nullptr without error for now (since I have no idea which element node is
a good node to return).

Then, we can rename it to GetSelectedElement() and can replace existing
GetSelectedElement() with the new one.

Comment 8

[Makoto Kato [:m_kato]]

Comment on attachment 8999591 [details]
Bug 1482019 - part 0: Add automated tests for nsIHTMLEditor.getSelectedElement()

Makoto Kato [:m_kato] has approved the revision.

Comment 9

[Makoto Kato [:m_kato]]

Comment on attachment 8999594 [details]
Bug 1482019 - part 1: Create non-virtual method HTMLEditor::GetSelectedNode() for implementing nsIHTMLEditor::GetSelectedElement()

Makoto Kato [:m_kato] has approved the revision.

Comment 10

[Makoto Kato [:m_kato]]

Comment on attachment 8999601 [details]
Bug 1482019 - part 2: Make HTMLEditor::GetSelectedNode() take nsAtom* for element name

Makoto Kato [:m_kato] has approved the revision.

Comment 11

[Makoto Kato [:m_kato]]

Comment on attachment 8999605 [details]
Bug 1482019 - part 3: Minimize some scope of auto varaiables in HTMLEditor::GetSelectedNode()

Makoto Kato [:m_kato] has approved the revision.

Comment 12

[Makoto Kato [:m_kato]]

Comment on attachment 8999608 [details]
Bug 1482019 - part 4: Reduce the indent level of the last block in HTMLEditor::GetSelectedNode()

Makoto Kato [:m_kato] has approved the revision.

Comment 13

[Makoto Kato [:m_kato]]

Comment on attachment 8999610 [details]
Bug 1482019 - part 5: Make HTMLEditor::GetSelectedNode() never return non-element node and change its name to GetSelectedElement()

Makoto Kato [:m_kato] has approved the revision.

Comment 14

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

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

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a61cdb1a654c
part 0: Add automated tests for nsIHTMLEditor.getSelectedElement() r=m_kato
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b46ca68f3b0
part 1: Create non-virtual method HTMLEditor::GetSelectedNode() for implementing nsIHTMLEditor::GetSelectedElement() r=m_kato
https://hg.mozilla.org/integration/mozilla-inbound/rev/40fbaeccbd89
part 2: Make HTMLEditor::GetSelectedNode() take nsAtom* for element name r=m_kato
https://hg.mozilla.org/integration/mozilla-inbound/rev/f6053ad1d49a
part 3: Minimize some scope of auto varaiables in HTMLEditor::GetSelectedNode() r=m_kato
https://hg.mozilla.org/integration/mozilla-inbound/rev/66e9a30a6b1a
part 4: Reduce the indent level of the last block in HTMLEditor::GetSelectedNode() r=m_kato
https://hg.mozilla.org/integration/mozilla-inbound/rev/57e568a7f933
part 5: Make HTMLEditor::GetSelectedNode() never return non-element node and change its name to GetSelectedElement() r=m_kato

Comment 16

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/a61cdb1a654c
https://hg.mozilla.org/mozilla-central/rev/9b46ca68f3b0
https://hg.mozilla.org/mozilla-central/rev/40fbaeccbd89
https://hg.mozilla.org/mozilla-central/rev/f6053ad1d49a
https://hg.mozilla.org/mozilla-central/rev/66e9a30a6b1a
https://hg.mozilla.org/mozilla-central/rev/57e568a7f933