Reviewed by John.
authormjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Sep 2004 19:26:53 +0000 (19:26 +0000)
committermjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Sep 2004 19:26:53 +0000 (19:26 +0000)
- renamed caretPos to caretRect and made it return a QRect instead
of taking four out parameters by reference.

        * khtml/rendering/render_box.cpp:
        (RenderBox::caretRect):
        * khtml/rendering/render_box.h:
        * khtml/rendering/render_br.cpp:
        (RenderBR::caretRect):
        * khtml/rendering/render_br.h:
        * khtml/rendering/render_flow.cpp:
        (RenderFlow::caretRect):
        * khtml/rendering/render_flow.h:
        * khtml/rendering/render_object.cpp:
        (RenderObject::caretRect):
        * khtml/rendering/render_object.h:
        * khtml/rendering/render_text.cpp:
        (RenderText::caretRect):
        * khtml/rendering/render_text.h:
        * khtml/xml/dom_selection.cpp:
        (DOM::Selection::xPosForVerticalArrowNavigation):
        (DOM::Selection::layoutCaret):
        * kwq/WebCoreBridge.mm:
        (-[WebCoreBridge caretRectAtNode:offset:]):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@7484 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
WebCore/ChangeLog-2005-08-23
WebCore/khtml/editing/SelectionController.cpp
WebCore/khtml/editing/selection.cpp
WebCore/khtml/rendering/render_box.cpp
WebCore/khtml/rendering/render_box.h
WebCore/khtml/rendering/render_br.cpp
WebCore/khtml/rendering/render_br.h
WebCore/khtml/rendering/render_flow.cpp
WebCore/khtml/rendering/render_flow.h
WebCore/khtml/rendering/render_object.cpp
WebCore/khtml/rendering/render_object.h
WebCore/khtml/rendering/render_text.cpp
WebCore/khtml/rendering/render_text.h
WebCore/khtml/xml/dom_selection.cpp
WebCore/kwq/WebCoreBridge.mm

index 7ef0162..4c2552a 100644 (file)
@@ -1,5 +1,33 @@
 2004-09-07  Maciej Stachowiak  <mjs@apple.com>
 
+        Reviewed by John.
+
+       - renamed caretPos to caretRect and made it return a QRect instead
+       of taking four out parameters by reference.
+       
+        * khtml/rendering/render_box.cpp:
+        (RenderBox::caretRect):
+        * khtml/rendering/render_box.h:
+        * khtml/rendering/render_br.cpp:
+        (RenderBR::caretRect):
+        * khtml/rendering/render_br.h:
+        * khtml/rendering/render_flow.cpp:
+        (RenderFlow::caretRect):
+        * khtml/rendering/render_flow.h:
+        * khtml/rendering/render_object.cpp:
+        (RenderObject::caretRect):
+        * khtml/rendering/render_object.h:
+        * khtml/rendering/render_text.cpp:
+        (RenderText::caretRect):
+        * khtml/rendering/render_text.h:
+        * khtml/xml/dom_selection.cpp:
+        (DOM::Selection::xPosForVerticalArrowNavigation):
+        (DOM::Selection::layoutCaret):
+        * kwq/WebCoreBridge.mm:
+        (-[WebCoreBridge caretRectAtNode:offset:]):
+
+2004-09-07  Maciej Stachowiak  <mjs@apple.com>
+
         Reviewed by Kevin.
 
        <rdar://problem/3759209> REGRESSION (Mail): assertion failure when all of marked text deleted at start of document
index 17a8182..b25200e 100644 (file)
@@ -394,8 +394,7 @@ int Selection::xPosForVerticalArrowNavigation(EPositionType type, bool recalc) c
         return x;
         
     if (recalc || part->xPosForVerticalArrowNavigation() == KHTMLPart::NoXPosForVerticalArrowNavigation) {
-        int y, w, h;
-        pos.node()->renderer()->caretPos(pos.offset(), true, x, y, w, h);
+        x = pos.node()->renderer()->caretRect(pos.offset(), true).x();
         part->setXPosForVerticalArrowNavigation(x);
     }
     else {
@@ -509,8 +508,10 @@ void Selection::layoutCaret()
     
     // EDIT FIXME: Enhance call to pass along selection 
     // upstream/downstream affinity to get the right position.
-    int w;
-    start().node()->renderer()->caretPos(start().offset(), true, m_caretX, m_caretY, w, m_caretSize);
+    QRect caretRect = start().node()->renderer()->caretRect(start().offset(), true);
+    m_caretX = caretRect.x();
+    m_caretY = caretRect.y();
+    m_caretSize = caretRect.height();
 
     m_needsCaretLayout = false;
 }
index 17a8182..b25200e 100644 (file)
@@ -394,8 +394,7 @@ int Selection::xPosForVerticalArrowNavigation(EPositionType type, bool recalc) c
         return x;
         
     if (recalc || part->xPosForVerticalArrowNavigation() == KHTMLPart::NoXPosForVerticalArrowNavigation) {
-        int y, w, h;
-        pos.node()->renderer()->caretPos(pos.offset(), true, x, y, w, h);
+        x = pos.node()->renderer()->caretRect(pos.offset(), true).x();
         part->setXPosForVerticalArrowNavigation(x);
     }
     else {
@@ -509,8 +508,10 @@ void Selection::layoutCaret()
     
     // EDIT FIXME: Enhance call to pass along selection 
     // upstream/downstream affinity to get the right position.
-    int w;
-    start().node()->renderer()->caretPos(start().offset(), true, m_caretX, m_caretY, w, m_caretSize);
+    QRect caretRect = start().node()->renderer()->caretRect(start().offset(), true);
+    m_caretX = caretRect.x();
+    m_caretY = caretRect.y();
+    m_caretSize = caretRect.height();
 
     m_needsCaretLayout = false;
 }
index b9d207c..260c80b 100644 (file)
@@ -1480,53 +1480,61 @@ void RenderBox::calcAbsoluteVertical()
 
 }
 
-void RenderBox::caretPos(int offset, bool override, int &_x, int &_y, int &width, int &height)
+QRect RenderBox::caretRect(int offset, bool override)
 {
-    _x = -1;
-    
+    // FIXME: Is it OK to check only first child instead of picking
+    // right child based on offset? Is it OK to pass the same offset
+    // along to the child instead of offset 0 or whatever?
+
     // propagate it downwards to its children, someone will feel responsible
     RenderObject *child = firstChild();
-    if (child) 
-        child->caretPos(offset, override, _x, _y, width, height);
+    if (child) {
+        QRect result = child->caretRect(offset, override);
+        // FIXME: in-band signalling!
+        if (result.x() != -1)
+            return result;
+    }
     
+    int _x, _y, width, height;
+
     // if not, use the extents of this box 
     // offset 0 means left, offset 1 means right
-    if (_x == -1) {
-        _x = xPos() + (offset == 0 ? 0 : m_width);
-        InlineBox *box = inlineBoxWrapper();
-        if (box) {
-            height = box->root()->bottomOverflow() - box->root()->topOverflow();
-            _y = box->root()->topOverflow();
-        }
-        else {
-            _y = yPos();
-            height = m_height;
-        }
-        width = override && offset == 0 ? m_width : 1;
-        // If height of box is smaller than font height, use the latter one,
-        // otherwise the caret might become invisible.
-        // 
-        // Also, if the box is not a replaced element, always use the font height.
-        // This prevents the "big caret" bug described in:
-        // <rdar://problem/3777804> Deleting all content in a document can result in giant tall-as-window insertion point
-        //
-        // FIXME: ignoring :first-line, missing good reason to take care of
-        int fontHeight = style()->fontMetrics().height();
-        if (fontHeight > height || !isReplaced())
-            height = fontHeight;
-        
-        int absx, absy;
-        RenderObject *cb = containingBlock();
-        if (cb && cb != this && cb->absolutePosition(absx,absy)) {
-            _x += absx;
-            _y += absy;
-        } 
-        else {
-            // we don't know our absolute position, and there is no point returning
-            // just a relative one
-            _x = _y = -1;
-        }
+    _x = xPos() + (offset == 0 ? 0 : m_width);
+    InlineBox *box = inlineBoxWrapper();
+    if (box) {
+        height = box->root()->bottomOverflow() - box->root()->topOverflow();
+        _y = box->root()->topOverflow();
+    }
+    else {
+        _y = yPos();
+        height = m_height;
     }
+    width = override && offset == 0 ? m_width : 1;
+    // If height of box is smaller than font height, use the latter one,
+    // otherwise the caret might become invisible.
+    // 
+    // Also, if the box is not a replaced element, always use the font height.
+    // This prevents the "big caret" bug described in:
+    // <rdar://problem/3777804> Deleting all content in a document can result in giant tall-as-window insertion point
+    //
+    // FIXME: ignoring :first-line, missing good reason to take care of
+    int fontHeight = style()->fontMetrics().height();
+    if (fontHeight > height || !isReplaced())
+        height = fontHeight;
+    
+    int absx, absy;
+    RenderObject *cb = containingBlock();
+    if (cb && cb != this && cb->absolutePosition(absx,absy)) {
+        _x += absx;
+        _y += absy;
+    } 
+    else {
+        // we don't know our absolute position, and there is no point returning
+        // just a relative one
+        _x = _y = -1;
+    }
+
+    return QRect(_x, _y, width, height);
 }
 
 int RenderBox::lowestPosition(bool includeOverflowInterior, bool includeSelf) const
index 3529bfb..bb84ff2 100644 (file)
@@ -130,7 +130,7 @@ public:
 
     virtual RenderLayer* layer() const { return m_layer; }
     
-    virtual void caretPos(int offset, bool override, int &_x, int &_y, int &width, int &height);
+    virtual QRect caretRect(int offset, bool override);
 
     virtual void paintBackgroundExtended(QPainter *p, const QColor &c, CachedImage *bg, int clipy, int cliph,
                                          int _tx, int _ty, int w, int height,
index b37d7ba..ceed127 100644 (file)
@@ -112,18 +112,17 @@ Position RenderBR::positionForCoordinates(int _x, int _y)
     return Position(element(), 0);
 }
 
-void RenderBR::caretPos(int offset, bool override, int &_x, int &_y, int &_w, int &_h)
+QRect RenderBR::caretRect(int offset, bool override)
 {
     // EDIT FIXME: This does not work yet. Some other changes are need before
     // an accurate position can be determined.
-    _h = lineHeight(false);
-    _x = xPos();
-    _y = yPos();
 
     int absx, absy;
     absolutePosition(absx,absy);
-    _x += absx;
-    _y += absy;
+
+    // FIXME: an older version of this code wasn't setting width at
+    // all, using the default of 1...
+    return QRect(xPos() + absx, yPos() + absy, 1, lineHeight(false));
 }
 
 InlineBox *RenderBR::inlineBox(long offset)
index b264a14..30b22ea 100644 (file)
@@ -69,7 +69,7 @@ public:
     virtual unsigned long caretMaxRenderedOffset() const;
     
     virtual DOM::Position positionForCoordinates(int _x, int _y);
-    virtual void caretPos(int offset, bool override, int &_x, int &_y, int &_w, int &_h);
+    virtual QRect caretRect(int offset, bool override);
 
     virtual InlineBox *inlineBox(long offset);
     
index 1703322..20d3446 100644 (file)
@@ -483,18 +483,19 @@ int RenderFlow::leftmostPosition(bool includeOverflowInterior, bool includeSelf)
     return left;
 }
 
-void RenderFlow::caretPos(int offset, bool override, int &_x, int &_y, int &width, int &height)
+QRect RenderFlow::caretRect(int offset, bool override)
 {
     if (firstChild() || style()->display() == INLINE) {
         // Do the normal calculation
-        RenderContainer::caretPos(offset, override, _x, _y, width, height);
-        return;
+        return RenderContainer::caretRect(offset, override);
     }
 
     // This is a special case:
     // The element is not an inline element, and it's empty. So we have to
     // calculate a fake position to indicate where objects are to be inserted.
     
+    int _x, _y, width, height;
+    
     // EDIT FIXME: this does neither take into regard :first-line nor :first-letter
     // However, as soon as some content is entered, the line boxes will be
     // constructed properly and this kludge is not called any more. So only
@@ -530,4 +531,6 @@ void RenderFlow::caretPos(int offset, bool override, int &_x, int &_y, int &widt
     absolutePosition(absx, absy, false);
     _x += absx + paddingLeft() + borderLeft();
     _y += absy + paddingTop() + borderTop();
+
+    return QRect(_x, _y, width, height);
 }
index cb0f83f..9815241 100644 (file)
@@ -81,7 +81,7 @@ public:
     virtual int rightmostPosition(bool includeOverflowInterior=true, bool includeSelf=true) const;
     virtual int leftmostPosition(bool includeOverflowInterior=true, bool includeSelf=true) const;
     
-    virtual void caretPos(int offset, bool override, int &_x, int &_y, int &width, int &height);
+    virtual QRect caretRect(int offset, bool override);
 
 protected:
     // An inline can be split with blocks occurring in between the inline content.
index 579a462..bb8015f 100644 (file)
@@ -1590,11 +1590,11 @@ bool RenderObject::absolutePosition(int &xPos, int &yPos, bool f)
     }
 }
 
-void RenderObject::caretPos(int /*offset*/, bool /*override*/, int &_x, int &_y, int &width, int &height)
+QRect RenderObject::caretRect(int /*offset*/, bool /*override*/)
 {
-    _x = _y = height = -1;
-    width = 1; // the caret has a default width of one pixel. If you want
-               // to check for validity, only test the x-coordinate for >= 0.
+    // the caret has a default width of one pixel. If you want
+    // to check for validity, only test the x-coordinate for >= 0.
+    return QRect(-1, -1, 1, -1);
 }
 
 int RenderObject::paddingTop() const
index 633d2ba..11d1e28 100644 (file)
@@ -708,12 +708,8 @@ public:
      * @param offset zero-based offset determining position within the render object.
      * @param override @p true if input overrides existing characters,
      * @p false if it inserts them. The width of the caret depends on this one.
-     * @param _x returns the left coordinate
-     * @param _y returns the top coordinate
-     * @param width returns the caret's width
-     * @param height returns the caret's height
      */
-    virtual void caretPos(int offset, bool override, int &_x, int &_y, int &width, int &height);
+    virtual QRect caretRect(int offset, bool override);
 
     virtual int lowestPosition(bool includeOverflowInterior=true, bool includeSelf=true) const { return 0; }
     virtual int rightmostPosition(bool includeOverflowInterior=true, bool includeSelf=true) const { return 0; }
index de9fc38..f0b9910 100644 (file)
@@ -650,11 +650,10 @@ Position RenderText::positionForCoordinates(int _x, int _y)
     return Position(element(), 0);
 }
 
-void RenderText::caretPos(int offset, bool override, int &_x, int &_y, int &width, int &height)
+QRect RenderText::caretRect(int offset, bool override)
 {
     if (!firstTextBox() || stringLength() == 0) {
-        _x = _y = height = -1;
-        return;
+        return QRect(-1, -1, 1, -1);
     }
 
     // Find the text box for the given offset
@@ -665,10 +664,11 @@ void RenderText::caretPos(int offset, bool override, int &_x, int &_y, int &widt
     }
     
     if (!box) {
-        _x = _y = height = -1;
-        return;
+        return QRect(-1, -1, 1, -1);
     }
 
+    int _x, _y, width, height;
+
     height = box->root()->bottomOverflow() - box->root()->topOverflow();
     _y = box->root()->topOverflow();
 
@@ -687,6 +687,8 @@ void RenderText::caretPos(int offset, bool override, int &_x, int &_y, int &widt
     absolutePosition(absx,absy);
     _x += absx;
     _y += absy;
+
+    return QRect(_x, _y, width, height);
 }
 
 void RenderText::posOfChar(int chr, int &x, int &y)
index 90dd36a..ced3252 100644 (file)
@@ -212,7 +212,7 @@ public:
 
     virtual SelectionState selectionState() const {return m_selectionState;}
     virtual void setSelectionState(SelectionState s) {m_selectionState = s; }
-    virtual void caretPos(int offset, bool override, int &_x, int &_y, int &width, int &height);
+    virtual QRect caretRect(int offset, bool override);
     void posOfChar(int ch, int &x, int &y);
 
     virtual short marginLeft() const { return style()->marginLeft().minWidth(0); }
index 17a8182..b25200e 100644 (file)
@@ -394,8 +394,7 @@ int Selection::xPosForVerticalArrowNavigation(EPositionType type, bool recalc) c
         return x;
         
     if (recalc || part->xPosForVerticalArrowNavigation() == KHTMLPart::NoXPosForVerticalArrowNavigation) {
-        int y, w, h;
-        pos.node()->renderer()->caretPos(pos.offset(), true, x, y, w, h);
+        x = pos.node()->renderer()->caretRect(pos.offset(), true).x();
         part->setXPosForVerticalArrowNavigation(x);
     }
     else {
@@ -509,8 +508,10 @@ void Selection::layoutCaret()
     
     // EDIT FIXME: Enhance call to pass along selection 
     // upstream/downstream affinity to get the right position.
-    int w;
-    start().node()->renderer()->caretPos(start().offset(), true, m_caretX, m_caretY, w, m_caretSize);
+    QRect caretRect = start().node()->renderer()->caretRect(start().offset(), true);
+    m_caretX = caretRect.x();
+    m_caretY = caretRect.y();
+    m_caretSize = caretRect.height();
 
     m_needsCaretLayout = false;
 }
index 89f9ef1..14a9b84 100644 (file)
@@ -1202,9 +1202,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
 
 - (NSRect)caretRectAtNode:(DOMNode *)node offset:(int)offset
 {
-    int x, y, w, h;
-    [node _nodeImpl]->renderer()->caretPos(offset, true, x, y, w, h);
-    return NSMakeRect(x, y, w, h);
+    return [node _nodeImpl]->renderer()->caretRect(offset, true);
 }
 
 - (NSImage *)selectionImage