[webkit-changes] cvs commit: WebCore/khtml/editing SelectionController.cpp composite_edit_command.cpp visible_units.cpp visible_units.h

Justin justing at opensource.apple.com
Fri Dec 9 18:59:32 PST 2005


justing     05/12/09 18:59:32

  Modified:    .        ChangeLog
               khtml/editing SelectionController.cpp
                        composite_edit_command.cpp visible_units.cpp
                        visible_units.h
  Log:
          <rdar://problem/4370209> Reproducible crash when pasting over whitespace:pre text
          <rdar://problem/4370220> Double or triple clicking in whitespace:pre text creates incorrect selections
          Some preparation for a fix for:
          <radar://problem/4364427> triple-click includes first item on next line (www.apple.com, but I think I've seen it elsewhere)
  
          There were a few bugs in endOfParagraph's and endOfLine's handling of IncludeLineBreak.
          The IncludeLinebreak concept also doesn't make sense: when asked to
          IncludeLineBreak, "endOfParagraph" would return the start of the next paragraph.
          Callers that want this funtionality should just call endOfParagraph and then get next()
          if it exists.
          In endOfParagraph's whitespace:pre handling, when the input visible position
          was at the end of a text node with whitespace:pre, that text node was searched
          for '/n'.  It should be skipped.
  
          Reviewed by harrison
  
          Added new layout tests in editing/pasting and editing/selection
  
          * khtml/editing/SelectionController.cpp:
          (khtml::SelectionController::validate):
          * khtml/editing/composite_edit_command.cpp:
          (khtml::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary):
          * khtml/editing/visible_units.cpp:
          (khtml::endOfLine):
          (khtml::isEndOfLine):
          (khtml::endOfParagraph):
          (khtml::isEndOfParagraph):
          * khtml/editing/visible_units.h:
  
  Revision  Changes    Path
  1.497     +31 -0     WebCore/ChangeLog
  
  Index: ChangeLog
  ===================================================================
  RCS file: /cvs/root/WebCore/ChangeLog,v
  retrieving revision 1.496
  retrieving revision 1.497
  diff -u -r1.496 -r1.497
  --- ChangeLog	9 Dec 2005 23:48:53 -0000	1.496
  +++ ChangeLog	10 Dec 2005 02:59:25 -0000	1.497
  @@ -1,3 +1,34 @@
  +2005-12-09  Justin Garcia  <justin.garcia at apple.com>
  +
  +        <rdar://problem/4370209> Reproducible crash when pasting over whitespace:pre text
  +        <rdar://problem/4370220> Double or triple clicking in whitespace:pre text creates incorrect selections
  +        Some preparation for a fix for:
  +        <radar://problem/4364427> triple-click includes first item on next line (www.apple.com, but I think I've seen it elsewhere)
  +        
  +        There were a few bugs in endOfParagraph's and endOfLine's handling of IncludeLineBreak.
  +        The IncludeLinebreak concept also doesn't make sense: when asked to 
  +        IncludeLineBreak, "endOfParagraph" would return the start of the next paragraph.  
  +        Callers that want this funtionality should just call endOfParagraph and then get next() 
  +        if it exists.
  +        In endOfParagraph's whitespace:pre handling, when the input visible position
  +        was at the end of a text node with whitespace:pre, that text node was searched
  +        for '/n'.  It should be skipped.
  +        
  +        Reviewed by harrison
  +
  +        Added new layout tests in editing/pasting and editing/selection
  +
  +        * khtml/editing/SelectionController.cpp:
  +        (khtml::SelectionController::validate):
  +        * khtml/editing/composite_edit_command.cpp:
  +        (khtml::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary):
  +        * khtml/editing/visible_units.cpp:
  +        (khtml::endOfLine):
  +        (khtml::isEndOfLine):
  +        (khtml::endOfParagraph):
  +        (khtml::isEndOfParagraph):
  +        * khtml/editing/visible_units.h:
  +
   2005-12-09  Tim Omernick  <timo at apple.com>
   
           Reviewed by Darin.
  
  
  
  1.106     +18 -3     WebCore/khtml/editing/SelectionController.cpp
  
  Index: SelectionController.cpp
  ===================================================================
  RCS file: /cvs/root/WebCore/khtml/editing/SelectionController.cpp,v
  retrieving revision 1.105
  retrieving revision 1.106
  diff -u -r1.105 -r1.106
  --- SelectionController.cpp	6 Dec 2005 04:29:24 -0000	1.105
  +++ SelectionController.cpp	10 Dec 2005 02:59:30 -0000	1.106
  @@ -906,17 +906,32 @@
               
               break;
               }
  -        case LINE:
  +        case LINE: {
  +            m_start = startOfLine(VisiblePosition(m_start, m_affinity)).deepEquivalent();
  +            VisiblePosition end = endOfLine(VisiblePosition(m_end, m_affinity));
  +            // If the end of this line is at the end of a paragraph, include the space 
  +            // after the end of the line in the selection.
  +            if (isEndOfParagraph(end)) {
  +                VisiblePosition next = end.next();
  +                if (next.isNotNull())
  +                    end = next;
  +            }
  +            m_end = end.deepEquivalent();
  +            break;
  +        }
           case LINE_BOUNDARY:
               m_start = startOfLine(VisiblePosition(m_start, m_affinity)).deepEquivalent();
  -            m_end = endOfLine(VisiblePosition(m_end, m_affinity), IncludeLineBreak).deepEquivalent();
  +            m_end = endOfLine(VisiblePosition(m_end, m_affinity)).deepEquivalent();
               break;
           case PARAGRAPH: {
               VisiblePosition pos(m_start, m_affinity);
               if (isStartOfLine(pos) && isEndOfDocument(pos))
                   pos = pos.previous();
               m_start = startOfParagraph(pos).deepEquivalent();
  -            m_end = endOfParagraph(VisiblePosition(m_end, m_affinity), IncludeLineBreak).deepEquivalent();
  +            VisiblePosition visibleParagraphEnd = endOfParagraph(VisiblePosition(m_end, m_affinity));
  +            // Include the space after the end of the paragraph in the selection.
  +            VisiblePosition startOfNextParagraph = visibleParagraphEnd.next();
  +            m_end = startOfNextParagraph.isNotNull() ? startOfNextParagraph.deepEquivalent() : visibleParagraphEnd.deepEquivalent();
               break;
           }
           case DOCUMENT_BOUNDARY:
  
  
  
  1.20      +11 -8     WebCore/khtml/editing/composite_edit_command.cpp
  
  Index: composite_edit_command.cpp
  ===================================================================
  RCS file: /cvs/root/WebCore/khtml/editing/composite_edit_command.cpp,v
  retrieving revision 1.19
  retrieving revision 1.20
  diff -u -r1.19 -r1.20
  --- composite_edit_command.cpp	21 Nov 2005 01:20:02 -0000	1.19
  +++ composite_edit_command.cpp	10 Dec 2005 02:59:31 -0000	1.20
  @@ -542,26 +542,29 @@
       
       VisiblePosition visiblePos(pos, VP_DEFAULT_AFFINITY);
       VisiblePosition visibleParagraphStart(startOfParagraph(visiblePos));
  -    VisiblePosition visibleParagraphEnd(endOfParagraph(visiblePos, IncludeLineBreak));
  +    VisiblePosition visibleParagraphEnd = endOfParagraph(visiblePos);
  +    VisiblePosition next = visibleParagraphEnd.next();
  +    VisiblePosition visibleEnd = next.isNotNull() ? next : visibleParagraphEnd;
  +    
       Position paragraphStart = visibleParagraphStart.deepEquivalent().upstream();
  -    Position paragraphEnd = visibleParagraphEnd.deepEquivalent().upstream();
  +    Position end = visibleEnd.deepEquivalent().upstream();
       
       // Perform some checks to see if we need to perform work in this function.
       if (paragraphStart.node()->isBlockFlow()) {
  -        if (paragraphEnd.node()->isBlockFlow()) {
  -            if (!paragraphEnd.node()->isAncestor(paragraphStart.node())) {
  +        if (end.node()->isBlockFlow()) {
  +            if (!end.node()->isAncestor(paragraphStart.node())) {
                   // If the paragraph end is a descendant of paragraph start, then we need to run
                   // the rest of this function. If not, we can bail here.
                   return;
               }
           }
  -        else if (paragraphEnd.node()->enclosingBlockFlowElement() != paragraphStart.node()) {
  +        else if (end.node()->enclosingBlockFlowElement() != paragraphStart.node()) {
               // The paragraph end is in another block that is an ancestor of the paragraph start.
               // We can bail as we have a full block to work with.
  -            ASSERT(paragraphStart.node()->isAncestor(paragraphEnd.node()->enclosingBlockFlowElement()));
  +            ASSERT(paragraphStart.node()->isAncestor(end.node()->enclosingBlockFlowElement()));
               return;
           }
  -        else if (isEndOfDocument(visibleParagraphEnd)) {
  +        else if (isEndOfDocument(visibleEnd)) {
               // At the end of the document. We can bail here as well.
               return;
           }
  @@ -572,7 +575,7 @@
       NodeImpl *moveNode = paragraphStart.node();
       if (paragraphStart.offset() >= paragraphStart.node()->caretMaxOffset())
           moveNode = moveNode->traverseNextNode();
  -    NodeImpl *endNode = paragraphEnd.node();
  +    NodeImpl *endNode = end.node();
       
       insertNodeAt(newBlock, paragraphStart.node(), paragraphStart.offset());
   
  
  
  
  1.45      +15 -35    WebCore/khtml/editing/visible_units.cpp
  
  Index: visible_units.cpp
  ===================================================================
  RCS file: /cvs/root/WebCore/khtml/editing/visible_units.cpp,v
  retrieving revision 1.44
  retrieving revision 1.45
  diff -u -r1.44 -r1.45
  --- visible_units.cpp	1 Dec 2005 10:32:15 -0000	1.44
  +++ visible_units.cpp	10 Dec 2005 02:59:31 -0000	1.45
  @@ -315,11 +315,8 @@
       return VisiblePosition(startNode, startOffset, DOWNSTREAM);
   }
   
  -VisiblePosition endOfLine(const VisiblePosition &c, EIncludeLineBreak includeLineBreak)
  +VisiblePosition endOfLine(const VisiblePosition &c)
   {
  -    // FIXME: Need to implement the "include line break" version.
  -    assert(includeLineBreak == DoNotIncludeLineBreak);
  -
       RootInlineBox *rootBox = rootBoxForLine(c);
       if (!rootBox)
           return VisiblePosition();
  @@ -343,8 +340,7 @@
           InlineTextBox *endTextBox = static_cast<InlineTextBox *>(endBox);
           endOffset = endTextBox->m_start + endTextBox->m_len;
       }
  -
  -    // generate VisiblePosition, use UPSTREAM affinity if possible
  +    
       return VisiblePosition(endNode, endOffset, VP_UPSTREAM_IF_POSSIBLE);
   }
   
  @@ -360,7 +356,7 @@
   
   bool isEndOfLine(const VisiblePosition &p)
   {
  -    return p.isNotNull() && p == endOfLine(p, DoNotIncludeLineBreak);
  +    return p.isNotNull() && p == endOfLine(p);
   }
   
   VisiblePosition previousLinePosition(const VisiblePosition &c, int x)
  @@ -587,16 +583,15 @@
       return VisiblePosition(node, offset, DOWNSTREAM);
   }
   
  -VisiblePosition endOfParagraph(const VisiblePosition &c, EIncludeLineBreak includeLineBreak)
  +VisiblePosition endOfParagraph(const VisiblePosition &c)
   {
  -    Position p = c.deepEquivalent();
  -
  -    NodeImpl *startNode = p.node();
  -    if (!startNode)
  +    if (c.isNull())
           return VisiblePosition();
   
  +    Position p = c.deepEquivalent();
  +    NodeImpl *startNode = p.node();
       NodeImpl *startBlock = startNode->enclosingBlockFlowElement();
  -    NodeImpl *stayInsideBlock = includeLineBreak ? 0 : startBlock;
  +    NodeImpl *stayInsideBlock = startBlock;
       
       NodeImpl *node = startNode;
       int offset = p.offset();
  @@ -610,42 +605,27 @@
           RenderStyle *style = r->style();
           if (style->visibility() != VISIBLE)
               continue;
  -        if (r->isBR()) {
  -            if (includeLineBreak) {
  -                VisiblePosition beforeBreak(n, 0, DOWNSTREAM);
  -                VisiblePosition next = beforeBreak.next();
  -                return next.isNotNull() ? next : beforeBreak;
  -            }
  -            break;
  -        }
  -        if (r->isBlockFlow()) {
  -            if (includeLineBreak)
  -                return VisiblePosition(n, 0, DOWNSTREAM);
  +            
  +        if (r->isBR() || r->isBlockFlow())
               break;
  -        }
  +            
           // FIXME: We avoid returning a position where the renderer can't accept the caret.
           // We should probably do this in other cases such as startOfParagraph.
           if (r->isText() && r->caretMaxRenderedOffset() > 0) {
  -            if (includeLineBreak && !n->isAncestor(startBlock))
  -                return VisiblePosition(n, 0, DOWNSTREAM);
               int length = static_cast<RenderText *>(r)->length();
               // FIXME: Not clear what to do with pre-wrap or pre-line here.
               if (style->whiteSpace() == PRE) {
                   QChar *text = static_cast<RenderText *>(r)->text();
  -                int o = 0;
  -                if (n == startNode && offset < length)
  -                    o = offset;
  +                int o = n == startNode ? offset : 0;
                   for (int i = o; i < length; ++i)
                       if (text[i] == '\n')
  -                        return VisiblePosition(n, i + includeLineBreak, DOWNSTREAM);
  +                        return VisiblePosition(n, i, DOWNSTREAM);
               }
               node = n;
  -            offset = length;
  +            offset = r->caretMaxOffset();
           } else if (r->isReplaced()) {
               node = n;
               offset = 1;
  -            if (includeLineBreak && !n->isAncestor(startBlock))
  -                break;
           }
       }
   
  @@ -664,7 +644,7 @@
   
   bool isEndOfParagraph(const VisiblePosition &pos)
   {
  -    return pos.isNotNull() && isEqualIgnoringAffinity(pos, endOfParagraph(pos, DoNotIncludeLineBreak));
  +    return pos.isNotNull() && isEqualIgnoringAffinity(pos, endOfParagraph(pos));
   }
   
   VisiblePosition previousParagraphPosition(const VisiblePosition &p, int x)
  
  
  
  1.11      +2 -3      WebCore/khtml/editing/visible_units.h
  
  Index: visible_units.h
  ===================================================================
  RCS file: /cvs/root/WebCore/khtml/editing/visible_units.h,v
  retrieving revision 1.10
  retrieving revision 1.11
  diff -u -r1.10 -r1.11
  --- visible_units.h	10 May 2005 04:03:34 -0000	1.10
  +++ visible_units.h	10 Dec 2005 02:59:31 -0000	1.11
  @@ -33,7 +33,6 @@
   class VisiblePosition;
   
   enum EWordSide { RightWordIfOnBoundary = false, LeftWordIfOnBoundary = true };
  -enum EIncludeLineBreak { DoNotIncludeLineBreak = false, IncludeLineBreak = true };
   
   // words
   VisiblePosition startOfWord(const VisiblePosition &, EWordSide = RightWordIfOnBoundary);
  @@ -43,7 +42,7 @@
   
   // lines
   VisiblePosition startOfLine(const VisiblePosition &);
  -VisiblePosition endOfLine(const VisiblePosition &, EIncludeLineBreak = DoNotIncludeLineBreak);
  +VisiblePosition endOfLine(const VisiblePosition &);
   VisiblePosition previousLinePosition(const VisiblePosition &, int x);
   VisiblePosition nextLinePosition(const VisiblePosition &, int x);
   bool inSameLine(const VisiblePosition &, const VisiblePosition &);
  @@ -58,7 +57,7 @@
   
   // paragraphs (perhaps a misnomer, can be divided by line break elements)
   VisiblePosition startOfParagraph(const VisiblePosition &);
  -VisiblePosition endOfParagraph(const VisiblePosition &, EIncludeLineBreak = DoNotIncludeLineBreak);
  +VisiblePosition endOfParagraph(const VisiblePosition &);
   VisiblePosition previousParagraphPosition(const VisiblePosition &, int x);
   VisiblePosition nextParagraphPosition(const VisiblePosition &, int x);
   bool inSameParagraph(const VisiblePosition &, const VisiblePosition &);
  
  
  



More information about the webkit-changes mailing list