[webkit-changes] cvs commit: WebCore/khtml/editing visible_position.cpp visible_units.cpp

Justin justing at opensource.apple.com
Mon Nov 28 17:35:08 PST 2005


justing     05/11/28 17:35:08

  Modified:    .        ChangeLog
               khtml/editing visible_position.cpp visible_units.cpp
  Log:
          <http://bugzilla.opendarwin.org/show_bug.cgi?id=5354>
          Corner case where you can select outside the bounds of an editable block.
  
          Reviewed by darin
  
          Updated/added editing test cases to reflect fix.
  
          * khtml/editing/visible_position.cpp:
          (khtml::VisiblePosition::init):
          It's ok to do some hunting to find a valid VisblePosition given a position
          that is not somewhere visible, like inside an empty div, but only leave the
          block containing the position used to construct the VisiblePosition if we
          can't find a VisiblePosition inside that block.
          We weren't using the equivalentDeepPosition of the input position to make
          the initUpstream/initDownstream decision, so we were sometimes moving
          past [br, 0], which is what having initUpstream around is supposed to prevent.
          I got rid of initUpstream and included the check to prevent moving
          past [br, 0] inside the now generalized init().
  
          * khtml/editing/visible_units.cpp:
          (khtml::endOfParagraph):
          After the changes to VisiblePosition::init(), asking for the visible position
          at  [br, 1] isn't the right way to include a line break.
  
  Revision  Changes    Path
  1.414     +26 -0     WebCore/ChangeLog
  
  Index: ChangeLog
  ===================================================================
  RCS file: /cvs/root/WebCore/ChangeLog,v
  retrieving revision 1.413
  retrieving revision 1.414
  diff -u -r1.413 -r1.414
  --- ChangeLog	28 Nov 2005 23:57:33 -0000	1.413
  +++ ChangeLog	29 Nov 2005 01:35:06 -0000	1.414
  @@ -1,3 +1,29 @@
  +2005-11-28  Justin Garcia  <justin.garcia at apple.com>
  +
  +        <http://bugzilla.opendarwin.org/show_bug.cgi?id=5354>
  +        Corner case where you can select outside the bounds of an editable block.
  +        
  +        Reviewed by darin
  +        
  +        Updated/added editing test cases to reflect fix.
  +        
  +        * khtml/editing/visible_position.cpp:
  +        (khtml::VisiblePosition::init):
  +        It's ok to do some hunting to find a valid VisblePosition given a position 
  +        that is not somewhere visible, like inside an empty div, but only leave the 
  +        block containing the position used to construct the VisiblePosition if we 
  +        can't find a VisiblePosition inside that block.
  +        We weren't using the equivalentDeepPosition of the input position to make 
  +        the initUpstream/initDownstream decision, so we were sometimes moving 
  +        past [br, 0], which is what having initUpstream around is supposed to prevent.  
  +        I got rid of initUpstream and included the check to prevent moving 
  +        past [br, 0] inside the now generalized init().
  +        
  +        * khtml/editing/visible_units.cpp:
  +        (khtml::endOfParagraph):
  +        After the changes to VisiblePosition::init(), asking for the visible position 
  +        at  [br, 1] isn't the right way to include a line break.
  +
   2005-11-28  Eric Seidel  <eseidel at apple.com>
   
           No review, build fix only.
  
  
  
  1.64      +21 -63    WebCore/khtml/editing/visible_position.cpp
  
  Index: visible_position.cpp
  ===================================================================
  RCS file: /cvs/root/WebCore/khtml/editing/visible_position.cpp,v
  retrieving revision 1.63
  retrieving revision 1.64
  diff -u -r1.63 -r1.64
  --- visible_position.cpp	21 Nov 2005 01:20:06 -0000	1.63
  +++ visible_position.cpp	29 Nov 2005 01:35:07 -0000	1.64
  @@ -70,63 +70,7 @@
   void VisiblePosition::init(const Position &pos, EAffinity affinity)
   {
       m_affinity = affinity;
  -
  -    // For <br> 0, it's important not to move past the <br>.
  -    if (pos.node() && pos.node()->hasTagName(brTag) && pos.offset() == 0)
  -        initUpstream(pos);
  -    else
  -        initDownstream(pos);
  -
  -    // when not at line break, make sure to end up with DOWNSTREAM affinity.  
  -    if (m_affinity == UPSTREAM && (isNull() || inSameLine(VisiblePosition(pos, DOWNSTREAM), *this)))
  -        m_affinity = DOWNSTREAM;
  -}
  -
  -void VisiblePosition::initUpstream(const Position &pos)
  -{
  -    Position deepPos = deepEquivalent(pos);
  -
  -    if (isCandidate(deepPos)) {
  -        m_deepPosition = deepPos;
  -        Position next = nextVisiblePosition(deepPos);
  -        if (next.isNotNull()) {
  -            Position previous = previousVisiblePosition(next);
  -            if (previous.isNotNull())
  -                m_deepPosition = previous;
  -        }
  -    }
  -    else {
  -        Position previous = previousVisiblePosition(deepPos);
  -        if (previous.isNotNull()) {
  -            m_deepPosition = previous;
  -        } else {
  -            Position next = nextVisiblePosition(deepPos);
  -            if (next.isNotNull())
  -                m_deepPosition = next;
  -        }
  -    }
  -}
  -
  -static bool isRenderedBR(NodeImpl *node)
  -{
  -    if (!node)
  -        return false;
  -
  -    RenderObject *renderer = node->renderer();
  -    if (!renderer)
  -        return false;
       
  -    if (renderer->style()->visibility() != VISIBLE)
  -        return false;
  -
  -    if (renderer->isBR())
  -        return true;
  -
  -    return false;
  -}
  -
  -void VisiblePosition::initDownstream(const Position &pos)
  -{
       Position deepPos = deepEquivalent(pos);
   
       if (isCandidate(deepPos)) {
  @@ -139,16 +83,30 @@
           }
       } else {
           Position next = nextVisiblePosition(deepPos);
  -        if (next.isNotNull()) {
  +        Position prev = previousVisiblePosition(deepPos);
  +        
  +        if (next.isNull() && prev.isNull())
  +            m_deepPosition = Position();
  +        else if (next.isNull())
  +            m_deepPosition = prev;
  +        else if (prev.isNull())
               m_deepPosition = next;
  -        } else if (isRenderedBR(deepPos.node()) && deepPos.offset() == 1) {
  -            m_deepPosition = Position(deepPos.node(), 0);
  -        } else {
  -            Position previous = previousVisiblePosition(deepPos);
  -            if (previous.isNotNull())
  -                m_deepPosition = previous;
  +        else {
  +            NodeImpl *originalBlock = pos.node() ? pos.node()->enclosingBlockFlowElement() : 0;
  +            bool nextIsInOriginalBlock = next.node()->isAncestor(originalBlock);
  +            bool prevIsInOriginalBlock = prev.node()->isAncestor(originalBlock);
  +            
  +            if (!nextIsInOriginalBlock && prevIsInOriginalBlock || 
  +                (deepPos.node()->hasTagName(brTag) && deepPos.offset() == 0))
  +                m_deepPosition = prev;
  +            else
  +                m_deepPosition = next;
           }
       }
  +
  +    // When not at a line wrap, make sure to end up with DOWNSTREAM affinity.
  +    if (m_affinity == UPSTREAM && (isNull() || inSameLine(VisiblePosition(pos, DOWNSTREAM), *this)))
  +        m_affinity = DOWNSTREAM;
   }
   
   VisiblePosition VisiblePosition::next() const
  
  
  
  1.43      +5 -2      WebCore/khtml/editing/visible_units.cpp
  
  Index: visible_units.cpp
  ===================================================================
  RCS file: /cvs/root/WebCore/khtml/editing/visible_units.cpp,v
  retrieving revision 1.42
  retrieving revision 1.43
  diff -u -r1.42 -r1.43
  --- visible_units.cpp	2 Nov 2005 08:52:43 -0000	1.42
  +++ visible_units.cpp	29 Nov 2005 01:35:07 -0000	1.43
  @@ -611,8 +611,11 @@
           if (style->visibility() != VISIBLE)
               continue;
           if (r->isBR()) {
  -            if (includeLineBreak)
  -                return VisiblePosition(n, 1, DOWNSTREAM);
  +            if (includeLineBreak) {
  +                VisiblePosition beforeBreak(n, 0, DOWNSTREAM);
  +                VisiblePosition next = beforeBreak.next();
  +                return next.isNotNull() ? next : beforeBreak;
  +            }
               break;
           }
           if (r->isBlockFlow()) {
  
  
  



More information about the webkit-changes mailing list