[webkit-reviews] review granted: [Bug 19221] ASSERT during Range creation (due to editing commands) : [Attachment 28043] First pass renaming

[bugzilla-daemon at webkit.org]

Justin Garcia <justin.garcia at apple.com> has granted Eric Seidel
<eric at webkit.org>'s request for review:
Bug 19221: ASSERT during Range creation (due to editing commands)
https://bugs.webkit.org/show_bug.cgi?id=19221

Attachment 28043: First pass renaming
https://bugs.webkit.org/attachment.cgi?id=28043&action=review

------- Additional Comments from Justin Garcia <justin.garcia at apple.com>
Don't really need to talk about WebCore code changes in the LayoutTest
ChangeLog.

+// Create a dom-compliant position which is outside of any ignored content

"DOM"

+// FIXME: This combines both avoiding tables and avoiding ignored nodes,
+// those should probably be separate functions.
+// FIXME: Split out part of this into positionAvoidingFirstPositionInTable?

These sound like the same FIXME.

+Position positionAvoidingIgnoredContent(const Position& originalPosition)
+{
+    Position position = originalPosition.toRangeCompliantEquivalent();

Should also mention that this function includes making positions DOM compliant,
which I believe is something you want to split out, no?

+	 I also cleaned up Position a little bit to make it a real class.

Please include your rationale for doing this, I remember Darin felt quite
strongly about Position being more like a struct.

+    // Returns node() cast to an element() or the closest element ancestor
+    Element* element() const;

"or the cloest element ancestor of node()."