Closed Bug 1359345 Opened 5 years ago Closed 5 years ago
Vis Break() should be renamed to HTMLEditor::Is Visible BRElement()
59 bytes, text/x-review-board-request
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.
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 firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/18f6b952efe9 HTMLEditor::IsVisBreak() should be renamed to HTMLEditor::IsVisibleBRElement(). r=masayuki
You need to log in before you can comment on or make changes to this bug.