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.
2 years ago
Depends on: 1359405
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.