Bug 25228 – SelectionController::absoluteCaretBounds returns an inflated caret (the caret repaint rect)

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Justin Garcia]

SelectionController::absoluteCaretBounds returns an inflated caret (the caret repaint rect)

Related to <rdar://problem/6590646>

[Justin Garcia]

I have a patch for this, working on a test case.

[Justin Garcia]

Created attachment 29526 [details]
patch

[Justin Garcia]

Created attachment 29527 [details]
patch

updated changelog

[Simon Fraser (smfr)]

Comment on attachment 29527 [details]
patch

> Index: WebCore/editing/SelectionController.cpp
> ===================================================================

> -    IntRect oldAbsRepaintRect = m_absCaretBounds;
> -    m_absCaretBounds = caretRepaintRect();
> +    IntRect oldAbsCaretBounds = m_absCaretBounds;
> +    m_absCaretBounds = m_caretRect;
> +    // m_caretRect is the local rect, we need the bounds of (possibly transformed) caret in absolute coordinates.
> +    RenderObject* caretPainter = caretRenderer();
> +    if (caretPainter)
> +        m_absCaretBounds = caretPainter->localToAbsoluteQuad(FloatRect(m_caretRect)).enclosingBoundingBox();
>      m_absCaretBoundsDirty = false;
>      
> -    if (oldAbsRepaintRect == m_absCaretBounds)
> +    if (oldAbsCaretBounds == m_absCaretBounds)
>          return false;
>      
>      if (RenderView* view = static_cast<RenderView*>(m_frame->document()->renderer())) {
>          // FIXME: make caret repainting container-aware.
> -        view->repaintRectangleInViewAndCompositedLayers(oldAbsRepaintRect, false);
> -        view->repaintRectangleInViewAndCompositedLayers(m_absCaretBounds, false);
> +        view->repaintRectangleInViewAndCompositedLayers(repaintRectForCaret(oldAbsCaretBounds), false);
> +        view->repaintRectangleInViewAndCompositedLayers(repaintRectForCaret(m_absCaretBounds), false);

I don't think this is right. This does the inflateX(1) after the transforms, whereas it needs to be before.

I think the correct solution is to make absoluteCaretBounds() look like caretRepaintRect(), without
doing the inflateX(1) via repaintRectForCaret.

Also, maybe we can get rid of the inflation entirely after bug 24527 is fixed? Bug 19086 needs re-testing.

[Justin Garcia]

Created attachment 29531 [details]
patch

[Simon Fraser (smfr)]

Comment on attachment 29531 [details]
patch

> Index: WebCore/editing/SelectionController.cpp
> ===================================================================

> +    IntRect oldAbsCaretBounds = m_absCaretBounds;
> +    // FIXME: Rename m_caretRect to m_localCaretRect.
> +    m_absCaretBounds = absoluteBoundsForLocalRect(m_caretRect);
>      
> -    if (oldAbsRepaintRect == m_absCaretBounds)
> +    if (oldAbsCaretBounds == m_absCaretBounds)
>          return false;
> +        
> +    IntRect oldAbsoluteCaretRepaintBounds = m_absoluteCaretRepaintBounds;
> +    // We believe that we need to inflate the local rect before transforming it to obtain the repaint bounds.
> +    m_absoluteCaretRepaintBounds = absoluteBoundsForLocalRect(repaintRectForCaret(m_caretRect));

You could just call caretRepaintRect() here.

r=me.

[Justin Garcia]

http://trac.webkit.org/changeset/42583