HTMLEditor::IsVisBreak() should be renamed to HTMLEditor::IsVisibleBRElement()

RESOLVED FIXED in Firefox 56

Status

()

defect
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: masayuki, Assigned: m_kato)

Tracking

(Blocks 1 bug)

Trunk
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 wontfix, firefox56 fixed)

Details

Attachments

(1 attachment)

HTMLEditor::IsVisBreak() isn't unclear to me. I think that IsVisibleBRElement() is clearer. So, I think that it should be renamed.

Additionally, it should be explained by comment like:

/**
 * Returns true if aNode is a <br> element and visible.  Otherwise, false.
 * Be aware, if aNode is a <br> element but it doesn't cause line break
 * actually, this method treats it as invisible.  E.g., a single line break
 * before a block boundary, i.e., <br> in "foo<p>bar<br></p>" and
 * "foo<br><p>bar</p>", is treated as invisible.
 */
Oh, and HTMLEditor::IsVisTextNode() should be renamed to IsVisibleTextNode() for the consistency.

/**
 * Returns true if aNode is a text node and visible. Otherwise, false.
 * When aSafeToAskFrames is true, this method checks if the text node is
 * actually mapped to a text frame.  Otherwise, this method checks if it
 * has non-whitespace characters.
 */

(Although, I don't understand well the detail of nsWSRunObject, so, the explanation when aSafeToAskFrames should have more detail if you know.)
Oh, the result of IsVisibleTextNode() is "isEmpty". So, the result is reverted. It's really error prone. Should be revert the meaning of result as the comment in this bug.
Priority: -- → P3
Assignee: nobody → m_kato
Comment on attachment 8888677 [details]
Bug 1359345 - HTMLEditor::IsVisBreak() should be renamed to HTMLEditor::IsVisibleBRElement().

https://reviewboard.mozilla.org/r/159694/#review165066
Attachment #8888677 - Flags: review?(masayuki) → review+
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/18f6b952efe9
HTMLEditor::IsVisBreak() should be renamed to HTMLEditor::IsVisibleBRElement(). r=masayuki
https://hg.mozilla.org/mozilla-central/rev/18f6b952efe9
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.