Changeset 13868 in webkit

Timestamp:

Apr 14, 2006 1:15:00 AM (18 years ago)

Author:

hyatt

Message:

Fix for 8333, make sure newlines in whitespace:pre (and friends) get
line boxes created for them. This resolves all the weird selection/navigation
issues that arise by not creating lines (and thus not having navigable positions
on those lines).

This checkin is also removing all of the layout test hacks that have piled
up, so layout test results are being regenerated completely.

Reviewed by eric

• dom/Position.cpp: (WebCore::Position::downstream):
• editing/CompositeEditCommand.cpp: (WebCore::CompositeEditCommand::moveParagraph):
• editing/DeleteSelectionCommand.cpp: (WebCore::DeleteSelectionCommand::doApply):
• editing/visible_units.cpp: (WebCore::startOfParagraph): (WebCore::endOfParagraph):
• kwq/RenderTreeAsText.cpp: (getTagName): (operator<<):
• rendering/InlineTextBox.cpp: (WebCore::InlineTextBox::selectionState): (WebCore::InlineTextBox::isLineBreak): (WebCore::InlineTextBox::nodeAtPoint): (WebCore::InlineTextBox::paint): (WebCore::InlineTextBox::offsetForPosition): (WebCore::InlineTextBox::positionForOffset):
• rendering/InlineTextBox.h:
• rendering/RenderBR.cpp:
• rendering/RenderBR.h:
• rendering/RenderText.cpp: (WebCore::RenderText::atLineWrap): (WebCore::RenderText::caretRect): (WebCore::RenderText::height): (WebCore::RenderText::inlineBox):
• rendering/bidi.cpp: (WebCore::RenderBlock::computeHorizontalPositionsForLine): (WebCore::RenderBlock::layoutInlineChildren): (WebCore::RenderBlock::findNextLineBreak):
• rendering/render_line.h: (WebCore::InlineBox::isLineBreak):

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Position.cpp (modified) (1 diff)
• editing/CompositeEditCommand.cpp (modified) (1 diff)
• editing/DeleteSelectionCommand.cpp (modified) (2 diffs)
• editing/visible_units.cpp (modified) (2 diffs)
• rendering/InlineTextBox.cpp (modified) (5 diffs)
• rendering/InlineTextBox.h (modified) (1 diff)
• rendering/RenderBR.cpp (modified) (1 diff)
• rendering/RenderBR.h (modified) (2 diffs)
• rendering/RenderText.cpp (modified) (4 diffs)
• rendering/RenderTreeAsText.cpp (modified) (2 diffs)
• rendering/bidi.cpp (modified) (8 diffs)
• rendering/render_line.h (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2006-04-14  David Hyatt  <hyatt@apple.com>
+
+        Fix for 8333, make sure newlines in whitespace:pre (and friends) get
+        line boxes created for them.  This resolves all the weird selection/navigation
+        issues that arise by not creating lines (and thus not having navigable positions
+        on those lines).
+
+        This checkin is also removing all of the layout test hacks that have piled
+        up, so layout test results are being regenerated completely.
+
+        Reviewed by eric
+
+        * dom/Position.cpp:
+        (WebCore::Position::downstream):
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::moveParagraph):
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::doApply):
+        * editing/visible_units.cpp:
+        (WebCore::startOfParagraph):
+        (WebCore::endOfParagraph):
+        * kwq/RenderTreeAsText.cpp:
+        (getTagName):
+        (operator<<):
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::selectionState):
+        (WebCore::InlineTextBox::isLineBreak):
+        (WebCore::InlineTextBox::nodeAtPoint):
+        (WebCore::InlineTextBox::paint):
+        (WebCore::InlineTextBox::offsetForPosition):
+        (WebCore::InlineTextBox::positionForOffset):
+        * rendering/InlineTextBox.h:
+        * rendering/RenderBR.cpp:
+        * rendering/RenderBR.h:
+        * rendering/RenderText.cpp:
+        (WebCore::RenderText::atLineWrap):
+        (WebCore::RenderText::caretRect):
+        (WebCore::RenderText::height):
+        (WebCore::RenderText::inlineBox):
+        * rendering/bidi.cpp:
+        (WebCore::RenderBlock::computeHorizontalPositionsForLine):
+        (WebCore::RenderBlock::layoutInlineChildren):
+        (WebCore::RenderBlock::findNextLineBreak):
+        * rendering/render_line.h:
+        (WebCore::InlineBox::isLineBreak):
+
 2006-04-13  Darin Adler  <darin@apple.com>
 

trunk/WebCore/dom/Position.cpp

@@ -433 +433 @@
                 
                 if (box != textRenderer->lastTextBox() && 
-                     !box->nextOnLine() && 
+                     !box->nextOnLine() &&
                      textOffset == box->start() + box->len()) {
                     return currentPos;

trunk/WebCore/editing/CompositeEditCommand.cpp

@@ -690 +690 @@
     
     // Deleting a paragraph leaves a placeholder (it always does when a whole paragraph is deleted).
-    // We remove it and prune it's parents since we want to remove all traces of the paragraph we're moving.
+    // We remove it and prune its parents since we want to remove all traces of the paragraph we're moving.
     Node* placeholder = endingSelection().end().node();
     if (placeholder->hasTagName(brTag))

trunk/WebCore/editing/DeleteSelectionCommand.cpp

@@ -601 +601 @@
         return;
 
-    // If the deletion is occuring in a text field, let the frame call across the bridge to notify the form delegate. 
+    // If the deletion is occurring in a text field, let the frame call across the bridge to notify the form delegate. 
     Node* startNode = m_selectionToDelete.start().node();
     Node* ancestorNode = startNode ? startNode->shadowAncestorNode() : 0;
@@ -627 +627 @@
                                isEndOfParagraph(VisiblePosition(m_downstreamEnd));
 
-    // Delete any text that may hinder our ability to fixup whitespace after the detele
+    // Delete any text that may hinder our ability to fixup whitespace after the delete
     deleteInsignificantTextDownstream(m_trailingWhitespace);    
 

trunk/WebCore/editing/visible_units.cpp

@@ -563 +563 @@
             break;
         if (r->isText()) {
-            // FIXME: Not clear what to do with pre-wrap or pre-line here.
-            if (style->whiteSpace() == PRE) {
+            if (style->preserveNewline()) {
                 const QChar* text = static_cast<RenderText*>(r)->text();
                 int i = static_cast<RenderText*>(r)->length();
@@ -624 +623 @@
         if (r->isText() && r->caretMaxRenderedOffset() > 0) {
             int length = static_cast<RenderText *>(r)->length();
-            // FIXME: Not clear what to do with pre-wrap or pre-line here.
-            if (style->whiteSpace() == PRE) {
+            if (style->preserveNewline()) {
                 const QChar* text = static_cast<RenderText *>(r)->text();
                 int o = n == startNode ? offset : 0;

trunk/WebCore/rendering/InlineTextBox.cpp

@@ -80 +80 @@
 {
     RenderObject::SelectionState state = object()->selectionState();
-    if (state == RenderObject::SelectionStart || state == RenderObject::SelectionEnd ||
-        state == RenderObject::SelectionBoth) {
+    if (state == RenderObject::SelectionStart || state == RenderObject::SelectionEnd || state == RenderObject::SelectionBoth) {
         int startPos, endPos;
         object()->selectionStartEnd(startPos, endPos);
         
+        // If we're at a line wrap, then the selection is going to extend onto the next line (and thus needs to be thought of as
+        // extending beyond our box.
+        if (textObject()->atLineWrap(this, endPos))
+            endPos++;
+
         bool start = (state != RenderObject::SelectionEnd && startPos >= m_start && startPos < m_start + m_len);
         bool end = (state != RenderObject::SelectionStart && endPos > m_start && endPos <= m_start + m_len);
@@ -219 +223 @@
 }
 
+bool InlineTextBox::isLineBreak() const
+{
+    return object()->isBR() || (object()->style()->preserveNewline() && len() == 1 && (*textObject()->string())[start()] == '\n');
+}
+
 bool InlineTextBox::nodeAtPoint(RenderObject::NodeInfo& i, int x, int y, int tx, int ty)
 {
-    if (object()->isBR())
+    if (isLineBreak())
         return false;
 
@@ -234 +243 @@
 void InlineTextBox::paint(RenderObject::PaintInfo& i, int tx, int ty)
 {
-    if (object()->isBR() || !object()->shouldPaintWithinRoot(i) || object()->style()->visibility() != VISIBLE ||
+    if (isLineBreak() || !object()->shouldPaintWithinRoot(i) || object()->style()->visibility() != VISIBLE ||
         m_truncation == cFullTruncation || i.phase == PaintPhaseOutline)
         return;
@@ -684 +693 @@
 int InlineTextBox::offsetForPosition(int _x, bool includePartialGlyphs) const
 {
+    if (isLineBreak())
+        return 0;
+
     RenderText* text = static_cast<RenderText*>(m_object);
     RenderStyle *style = text->style(m_firstLine);
@@ -694 +706 @@
 int InlineTextBox::positionForOffset(int offset) const
 {
+    if (isLineBreak())
+        return m_x;
+
     RenderText *text = static_cast<RenderText *>(m_object);
     const Font *f = text->font(m_firstLine);

trunk/WebCore/rendering/InlineTextBox.h

@@ -86 +86 @@
     virtual int placeEllipsisBox(bool ltr, int blockEdge, int ellipsisWidth, bool& foundBox);
 
+    virtual bool isLineBreak() const;
+
     // Overloaded new operator.  Derived classes must override operator new
     // in order to allocate out of the RenderArena.

trunk/WebCore/rendering/RenderBR.cpp

@@ -108 +108 @@
 }
 
-IntRect RenderBR::caretRect(int offset, EAffinity affinity, int *extraWidthToEndOfLine)
-{
-    // EDIT FIXME: This does not work yet. Some other changes are need before
-    // an accurate position can be determined.
-
-    int absx, absy;
-    absolutePosition(absx, absy);
-
-    if (extraWidthToEndOfLine)
-        *extraWidthToEndOfLine = containingBlockWidth() - xPos();
- 
-    // FIXME: an older version of this code wasn't setting width at
-    // all, using the default of 1...
-    return IntRect(xPos() + absx, yPos() + absy, 1, lineHeight(false));
-}
-
 InlineBox *RenderBR::inlineBox(int offset, EAffinity affinity)
 {

trunk/WebCore/rendering/RenderBR.h

@@ -54 +54 @@
     // overrides
     virtual InlineBox* createInlineBox(bool, bool, bool isOnlyRun = false);
-    virtual void calcMinMaxWidth() {}
-    virtual int minWidth() const { return 0; }
-    virtual int maxWidth() const { return 0; }
 
     virtual bool isBR() const { return true; }
@@ -65 +62 @@
     
     virtual VisiblePosition positionForCoordinates(int x, int y);
-    virtual IntRect caretRect(int offset, EAffinity affinity = UPSTREAM, int *extraWidthToEndOfLine = 0);
 
     virtual InlineBox *inlineBox(int offset, EAffinity affinity = UPSTREAM);

trunk/WebCore/rendering/RenderText.cpp

@@ -352 +352 @@
 {
     if (box->nextTextBox() && !box->nextOnLine() && offset == box->m_start + box->m_len) {
-        // Take special care because in preformatted text, the newlines
-        // are in between the text boxes (i.e. not in any box's m_start
-        // thru m_start+m_len-1), even though they are rendered.
-        if (!style()->preserveNewline() || (*str)[offset] != '\n')
+        if (!style()->preserveNewline() || box->isLineBreak())
             return true;
     }
@@ -372 +369 @@
         if ((offset >= box->m_start) && (offset <= box->m_start + box->m_len)) {
             // Check if downstream affinity would make us move to the next line.
-            if (affinity == DOWNSTREAM && atLineWrap(box, offset)) {
+            if (atLineWrap(box, offset)) {
                 // Use the next text box
                 box = box->nextTextBox();
@@ -920 +917 @@
 int RenderText::height() const
 {
-    // FIXME: Why use line-height? Shouldn't we be adding in the height of the last text box? -dwh
     int retval = 0;
     if (firstTextBox())
-        retval = lastTextBox()->m_y + lineHeight(false) - firstTextBox()->m_y;
+        retval = lastTextBox()->m_y + lastTextBox()->height() - firstTextBox()->m_y;
     return retval;
 }
@@ -1112 +1108 @@
     for (InlineTextBox *box = firstTextBox(); box; box = box->nextTextBox()) {
         if (offset >= box->m_start && offset <= box->m_start + box->m_len) {
-            if (affinity == DOWNSTREAM && atLineWrap(box, offset))
+            if (atLineWrap(box, offset))
                 return box->nextTextBox();
             return box;

trunk/WebCore/rendering/RenderTreeAsText.cpp

@@ -103 +103 @@
     if (n->isDocumentNode())
         return "";
-    if (n->isTextNode())
-        return "TEXT"; // FIXME: Remove once the layout tests are ready to change.
     if (n->isCommentNode())
         return "COMMENT";
-    if (n->isHTMLElement())
-        return n->nodeName().upper().deprecatedString(); // FIXME: We want to dump the real DOM name, not an uppercase name.
     return n->nodeName().deprecatedString(); 
 }
@@ -127 +123 @@
     }
     
-    // FIXME: Will remove this <br> code once all layout tests pass.  Until then, we can't really change
-    // all the results easily.
-    // FIXME: Will also remove the table row and section hacks once all layout tests pass.
-    bool usePositions = true;
-    bool useWidth = true;
-    bool useHeight = true;
-    if (o.isBR()) {
-        const RenderBR* br = static_cast<const RenderBR*>(&o);
-        usePositions = (br->firstTextBox() && br->firstTextBox()->isText());
-    } else if (o.isTableRow())
-        usePositions = useWidth = useHeight = false;
-    else if (o.isTableSection())
-        useWidth = false;
-
-    IntRect r(usePositions ? o.xPos() : 0, usePositions ? o.yPos() : 0, useWidth ? o.width() : 0, useHeight ? o.height() : 0);
+    IntRect r(o.xPos(), o.yPos(), o.width(), o.height());
     ts << " " << r;
     

trunk/WebCore/rendering/bidi.cpp

@@ -102 +102 @@
 static bool isLineEmpty = true;
 static bool previousLineBrokeCleanly = true;
-static bool skipTrailingNewline = false;
 static bool emptyRun = true;
 static int numSpaces;
@@ -855 +854 @@
     bool needsWordSpacing = false;
     for (r = sFirstBidiRun; r; r = r->nextRun) {
-        if (!r->box || r->obj->isPositioned())
+        if (!r->box || r->obj->isPositioned() || r->box->isLineBreak())
             continue; // Positioned objects are only participating to figure out their
                       // correct static x position.  They have no effect on the width.
+                      // Similarly, line break boxes have no effect on the width.
         if (r->obj->isText()) {
             RenderText *rt = static_cast<RenderText *>(r->obj);
@@ -1631 +1631 @@
                 }
                 
-                if (end == start || skipTrailingNewline) {
+                if (end == start) {
                     bidi.adjustEmbedding = true;
                     end.increment(bidi);
@@ -1975 +1975 @@
     bool prevLineBrokeCleanly = previousLineBrokeCleanly;
     previousLineBrokeCleanly = false;
-    skipTrailingNewline = false;
     
     while (o) {
@@ -2187 +2186 @@
 
                 if (c == '\n' || (o->style()->whiteSpace() != PRE && isBreakable(str, pos, strlen, nextBreakable, breakNBSP)) || midWordBreak) {
+                    bool stoppedIgnoringSpaces = false;
                     if (ignoringSpaces) {
                         if (!currentCharacterIsSpace) {
@@ -2196 +2196 @@
                             BidiIterator startMid ( 0, o, pos );
                             addMidpoint(startMid);
+                            stoppedIgnoringSpaces = true;
                         } else {
                             // Just keep ignoring these spaces.
@@ -2250 +2251 @@
                             }
                             if (lBreak.obj && lBreak.obj->style()->preserveNewline() && lBreak.obj->isText() && static_cast<RenderText*>(lBreak.obj)->text()[lBreak.pos] == '\n') {
+                                if (!stoppedIgnoringSpaces && pos > 0) {
+                                    // We need to stop right before the newline and then start up again.
+                                    BidiIterator midpoint(0, o, pos);
+                                    addMidpoint(BidiIterator(0, o, pos-1)); // Stop
+                                    addMidpoint(BidiIterator(0, o, pos)); // Start
+                                }
+                                lBreak.increment(bidi);
                                 previousLineBrokeCleanly = true;
-                                skipTrailingNewline = true;
                             }
                             goto end; // Didn't fit. Jump to the end.
@@ -2264 +2271 @@
 
                     if (c == '\n' && o->style()->preserveNewline()) {
+                        if (!stoppedIgnoringSpaces && pos > 0) {
+                            // We need to stop right before the newline and then start up again.
+                            BidiIterator midpoint(0, o, pos);
+                            addMidpoint(BidiIterator(0, o, pos-1)); // Stop
+                            addMidpoint(BidiIterator(0, o, pos)); // Start
+                        }
                         lBreak.obj = o;
                         lBreak.pos = pos;
+                        lBreak.increment(bidi);
                         previousLineBrokeCleanly = true;
-                        skipTrailingNewline = true;
                         return lBreak;
                     }

trunk/WebCore/rendering/render_line.h

@@ -60 +60 @@
     virtual void attachLine();
 
+    virtual bool isLineBreak() const { return false; }
+
     virtual void adjustPosition(int dx, int dy);