1359404 - HTMLEditor::IsEmptyNode() and HTMLEditor::IsEmptyNodeImpl() should return bool

Status: RESOLVED FIXED

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

Description

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

HTMLEditor::IsEmptyNode() and HTMLEditor::IsEmptyNodeImpl() returns nsresult. However, the error result isn't useful for callers since they return error only when some information of editor isn't available, e.g., selection controller isn't available.

So, just returning bool value is fine.

Comment 1

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

Ah, and perhaps, IsEmptyNodeImpl() should return a struct which has mIsEmpty and mHasBRElement. Then, we can improve the complicated arguments.

Comment 2

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

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

Comment 3

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

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

Comment 4

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

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

Comment 5

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

Attachment: Bug 1359404 - Redesign HTMLEditor::IsVisTextNode()

Despite of its name, HTMLEditor::IsVisTextNode() returns true with its out
argument when the given text node is empty.  And although it returns nsresult,
it's almost always NS_OK because it returns error only when the editor isn't
usual condition.

So, it should return bool and true when the given text node is visible.

This patch separates the method.  One is for checking the node with frames,
called IsInVisibleTextFrames().  The other is for checking it only with
its text, called IsVisibleTextNode().

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

Comment 6

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

Comment on attachment 8938000 [details]
Bug 1359404 - Redesign HTMLEditor::IsVisTextNode()

Oops, the bug# in the commit message is wrong.

Comment 7

[Makoto Kato [:m_kato]]

Attachment: Bug 1359404 - Make IsEmptyNode infaillible.

IsEmptyNode won't return error if parameter is not null. So we shouldn't use
nserror for return value and use assertion to check parameter.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/808290d9cd09
Make IsEmptyNode infaillible. r=masayuki

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/808290d9cd09