Commit 0f8de539 authored by eric@webkit.org's avatar eric@webkit.org
Browse files

Reviewed by Darin Adler.

        Rename Position::container to m_anchorNode and make it private
        https://bugs.webkit.org/show_bug.cgi?id=24760

        More code cleanup for Position.

        Change all uses of m_container to node()
        Eventually most uses of node() should change to anchorNode() to designate
        that it's the node the Position is anchored to, but not necessarily the
        container of the position (it could be the before/after neighbor).

        Remove any code which sets m_container, and change it to use a new
        Position::moveToPosition function which takes a node and offset.
        It never makes sense to change the node and leave the offset.

        * dom/Position.h:
        (WebCore::Position::Position):
        (WebCore::Position::clear):
        (WebCore::Position::anchorNode):
        (WebCore::Position::node):
        (WebCore::Position::moveToPosition):
        (WebCore::Position::moveToOffset):
        (WebCore::Position::isNull):
        (WebCore::Position::isNotNull):
        (WebCore::operator==):
        * dom/Range.cpp:
        (WebCore::Range::create):
        (WebCore::Range::compareBoundaryPoints):
        * dom/RangeBoundaryPoint.h:
        (WebCore::RangeBoundaryPoint::container):
        (WebCore::RangeBoundaryPoint::set):
        (WebCore::RangeBoundaryPoint::setOffset):
        (WebCore::RangeBoundaryPoint::setToChild):
        (WebCore::RangeBoundaryPoint::setToStart):
        (WebCore::RangeBoundaryPoint::setToEnd):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@41933 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 4732c307
2009-03-23 Eric Seidel <eric@webkit.org>
Reviewed by Darin Adler.
Rename Position::container to m_anchorNode and make it private
https://bugs.webkit.org/show_bug.cgi?id=24760
More code cleanup for Position.
Change all uses of m_container to node()
Eventually most uses of node() should change to anchorNode() to designate
that it's the node the Position is anchored to, but not necessarily the
container of the position (it could be the before/after neighbor).
Remove any code which sets m_container, and change it to use a new
Position::moveToPosition function which takes a node and offset.
It never makes sense to change the node and leave the offset.
* dom/Position.h:
(WebCore::Position::Position):
(WebCore::Position::clear):
(WebCore::Position::anchorNode):
(WebCore::Position::node):
(WebCore::Position::moveToPosition):
(WebCore::Position::moveToOffset):
(WebCore::Position::isNull):
(WebCore::Position::isNotNull):
(WebCore::operator==):
* dom/Range.cpp:
(WebCore::Range::create):
(WebCore::Range::compareBoundaryPoints):
* dom/RangeBoundaryPoint.h:
(WebCore::RangeBoundaryPoint::container):
(WebCore::RangeBoundaryPoint::set):
(WebCore::RangeBoundaryPoint::setOffset):
(WebCore::RangeBoundaryPoint::setToChild):
(WebCore::RangeBoundaryPoint::setToStart):
(WebCore::RangeBoundaryPoint::setToEnd):
2009-03-17 Eric Seidel <eric@webkit.org>
 
Reviewed by David Hyatt.
......@@ -52,19 +52,36 @@ enum PositionMoveType {
class Position {
public:
RefPtr<Node> container;
int m_offset;
Position() : m_offset(0) { }
Position(PassRefPtr<Node> c, int o) : container(c), m_offset(o) { }
void clear() { container.clear(); m_offset = 0; }
// This constructor should be private
Position(PassRefPtr<Node> anchorNode, int offset)
: m_anchorNode(anchorNode)
, m_offset(offset)
{}
void clear() { m_anchorNode.clear(); m_offset = 0; }
Node* anchorNode() const { return m_anchorNode.get(); }
Node* node() const { return container.get(); }
// FIXME: Callers should be moved off of node(), node() is not always the container for this position.
// For nodes which editingIgnoresContent(node()) returns true, positions like [ignoredNode, 0]
// will be treated as before ignoredNode (thus node() is really after the position, not containing it).
Node* node() const { return m_anchorNode.get(); }
Element* documentElement() const;
bool isNull() const { return !container; }
bool isNotNull() const { return container; }
void moveToPosition(PassRefPtr<Node> node, int offset)
{
m_anchorNode = node;
m_offset = offset;
}
void moveToOffset(int offset)
{
m_offset = offset;
}
bool isNull() const { return !m_anchorNode; }
bool isNotNull() const { return m_anchorNode; }
Element* element() const;
PassRefPtr<CSSComputedStyleDeclaration> computedStyle() const;
......@@ -118,11 +135,20 @@ private:
Position previousCharacterPosition(EAffinity) const;
Position nextCharacterPosition(EAffinity) const;
RefPtr<Node> m_anchorNode;
public:
// m_offset can be the offset inside m_anchorNode, or if editingIgnoresContent(m_anchorNode)
// returns true, then other places in editing will treat m_offset == 0 as "before the anchor"
// and m_offset > 0 as "after the anchor node". See rangeCompliantEquivalent for more info.
int m_offset; // FIXME: This should be made private.
};
inline bool operator==(const Position& a, const Position& b)
{
return a.container == b.container && a.m_offset == b.m_offset;
// FIXME: In <div><img></div> [div, 0] != [img, 0] even though most of the
// editing code will treat them as identical.
return a.anchorNode() == b.anchorNode() && a.m_offset == b.m_offset;
}
inline bool operator!=(const Position& a, const Position& b)
......
......@@ -89,7 +89,7 @@ PassRefPtr<Range> Range::create(PassRefPtr<Document> ownerDocument, PassRefPtr<N
PassRefPtr<Range> Range::create(PassRefPtr<Document> ownerDocument, const Position& start, const Position& end)
{
return adoptRef(new Range(ownerDocument, start.container.get(), start.m_offset, end.container.get(), end.m_offset));
return adoptRef(new Range(ownerDocument, start.node(), start.m_offset, end.node(), end.m_offset));
}
Range::~Range()
......@@ -527,7 +527,7 @@ short Range::compareBoundaryPoints(Node* containerA, int offsetA, Node* containe
short Range::compareBoundaryPoints(const Position& a, const Position& b)
{
return compareBoundaryPoints(a.container.get(), a.m_offset, b.container.get(), b.m_offset);
return compareBoundaryPoints(a.node(), a.m_offset, b.node(), b.m_offset);
}
bool Range::boundaryPointsValid() const
......
......@@ -55,6 +55,10 @@ public:
private:
static const int invalidOffset = -1;
// FIXME: RangeBoundaryPoint is the only file to ever use -1 as am expected offset for Position
// RangeBoundaryPoint currently needs to store a Position object to make the
// position() function be able to return a const& (and thus avoid ref-churn).
mutable Position m_position;
Node* m_childBefore;
};
......@@ -72,7 +76,7 @@ inline RangeBoundaryPoint::RangeBoundaryPoint(PassRefPtr<Node> container)
inline Node* RangeBoundaryPoint::container() const
{
return m_position.container.get();
return m_position.node();
}
inline Node* RangeBoundaryPoint::childBefore() const
......@@ -104,47 +108,43 @@ inline void RangeBoundaryPoint::set(PassRefPtr<Node> container, int offset, Node
{
ASSERT(offset >= 0);
ASSERT(childBefore == (offset ? container->childNode(offset - 1) : 0));
m_position.container = container;
m_position.m_offset = offset;
m_position.moveToPosition(container, offset);
m_childBefore = childBefore;
}
inline void RangeBoundaryPoint::setOffset(int offset)
{
ASSERT(m_position.container);
ASSERT(m_position.container->offsetInCharacters());
ASSERT(m_position.node());
ASSERT(m_position.node()->offsetInCharacters());
ASSERT(m_position.m_offset >= 0);
ASSERT(!m_childBefore);
m_position.m_offset = offset;
m_position.moveToOffset(offset);
}
inline void RangeBoundaryPoint::setToChild(Node* child)
{
ASSERT(child);
ASSERT(child->parentNode());
m_position.container = child->parentNode();
m_childBefore = child->previousSibling();
m_position.m_offset = m_childBefore ? invalidOffset : 0;
m_position.moveToPosition(child->parentNode(), m_childBefore ? invalidOffset : 0);
}
inline void RangeBoundaryPoint::setToStart(PassRefPtr<Node> container)
{
ASSERT(container);
m_position.container = container;
m_position.m_offset = 0;
m_position.moveToPosition(container, 0);
m_childBefore = 0;
}
inline void RangeBoundaryPoint::setToEnd(PassRefPtr<Node> container)
{
ASSERT(container);
m_position.container = container;
if (m_position.container->offsetInCharacters()) {
m_position.m_offset = m_position.container->maxCharacterOffset();
if (container->offsetInCharacters()) {
m_position.moveToPosition(container, container->maxCharacterOffset());
m_childBefore = 0;
} else {
m_childBefore = m_position.container->lastChild();
m_position.m_offset = m_childBefore ? invalidOffset : 0;
m_childBefore = container->lastChild();
m_position.moveToPosition(container, m_childBefore ? invalidOffset : 0);
}
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment