[webkit-reviews] review granted: [Bug 24760] Clean up Position.h : [Attachment 28855] Remove Position::clear() and make VisibleSelection.h not include VisiblePosition.h

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 23 09:55:00 PDT 2009


Darin Adler <darin at apple.com> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 24760: Clean up Position.h
https://bugs.webkit.org/show_bug.cgi?id=24760

Attachment 28855: Remove Position::clear() and make VisibleSelection.h not
include VisiblePosition.h
https://bugs.webkit.org/attachment.cgi?id=28855&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +
> +    // Returns node() or the closest element accestor
> +    Element* element() const;
>      Element* documentElement() const;

The paragraphing here makes it look like this comment is for both element() and
documentElement(). The comment also seems to be unclear about what this will
return when node() is not an element, but there is no closest element ancestor.
The answer is "0", but the comment makes it sound like we'd return node(),
which isn't possible since it's not an Element*.

Why are you removing clear()? I like clear(), and all the functions where
you’re removing it seem, well, less clear after the change. What does removing
it have to do with the other changes you’re planning?

r=me, but really?


More information about the webkit-reviews mailing list