Closed
Bug 1359345
Opened 8 years ago
Closed 7 years ago
HTMLEditor::IsVisBreak() should be renamed to HTMLEditor::IsVisibleBRElement()
Categories
(Core :: DOM: Editor, defect, P3)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
mozilla56
People
(Reporter: masayuki, Assigned: m_kato)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
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.
*/
Reporter | ||
Comment 1•8 years ago
|
||
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.)
Reporter | ||
Comment 2•8 years ago
|
||
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.
Updated•8 years ago
|
status-firefox55:
--- → affected
status-firefox57:
affected → ---
Assignee | ||
Updated•7 years ago
|
Priority: -- → P3
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → m_kato
Comment hidden (mozreview-request) |
Reporter | ||
Comment 4•7 years ago
|
||
mozreview-review |
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
Comment 6•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•