Bug 34923 - Incorrect client rects for blocks the span multiple columns and their descendants
Summary: Incorrect client rects for blocks the span multiple columns and their descend...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-02-13 17:29 PST by mitz
Modified: 2010-02-15 11:49 PST (History)
2 users (show)

See Also:


Attachments
Test case (1.15 KB, text/html)
2010-02-13 17:29 PST, mitz
no flags Details
WIP (15.88 KB, patch)
2010-02-14 00:51 PST, mitz
no flags Details | Formatted Diff | Diff
Allow offsetFromContainer() to return the right offset for a given point in the multi-column case, and pass it the point of interest (53.84 KB, patch)
2010-02-14 13:56 PST, mitz
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2010-02-13 17:29:50 PST
Created attachment 48712 [details]
Test case

The DOM getClientRects() API and the -[DOMRange textRects] method return incorrect results for blocks and their descendants when those span more than one column. The attached test case gives an example with getClientRects().
Comment 1 mitz 2010-02-13 17:30:20 PST
<rdar://problem/7647300>
Comment 2 mitz 2010-02-14 00:51:54 PST
Created attachment 48716 [details]
WIP
Comment 3 mitz 2010-02-14 13:56:27 PST
Created attachment 48732 [details]
Allow offsetFromContainer() to return the right offset for a given point in the multi-column case, and pass it the point of interest

This is obviously just a better approximation of correct behavior in the multi-column case, as it still assumes that a quad is mapped to absolute coordinates via a continuous transform. In the multi-column case, the transform is only piecewise-continuous. So this just picks the piece that applies to some point in the quad, rather than always picking the first piece.
Comment 4 Simon Fraser (smfr) 2010-02-15 10:02:08 PST
Comment on attachment 48732 [details]
Allow offsetFromContainer() to return the right offset for a given point in the multi-column case, and pass it the point of interest

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 54757)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,63 @@
> +2010-02-14  Dan Bernstein  <mitz@apple.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        <rdar://problem/7647300> Incorrect client rects for blocks the span multiple columns and their descendants
> +        https://bugs.webkit.org/show_bug.cgi?id=34923
> +

I'd like to see a summary of the problem, and your general approach at fixing it here.

> +        (WebCore::SelectionController::layout): Pass the caretâs origin as the

Garbled curly quote.

> +        column offset to the computation, which previously this function didnât

Ditto

> +        TransformState with the top left corner of the quadâs bounding box. It

And again

> Index: WebCore/rendering/RenderObject.h
> ===================================================================
> --- WebCore/rendering/RenderObject.h	(revision 54757)
> +++ WebCore/rendering/RenderObject.h	(working copy)
> @@ -563,7 +563,7 @@ public:
>      FloatQuad localToContainerQuad(const FloatQuad&, RenderBoxModelObject* repaintContainer, bool fixed = false) const;
>  
>      // Return the offset from the container() renderer (excluding transforms)
> -    virtual IntSize offsetFromContainer(RenderObject*) const;
> +    virtual IntSize offsetFromContainer(RenderObject*, const IntPoint&) const;

I'd like the comment to explain what the point is for.

> +    virtual void adjustForColumns(IntSize&, const IntPoint&) const { }

Also here. How is the IntSize being adjusted? What does the point represent?

r=me, with some addition commentary.
Comment 5 mitz 2010-02-15 11:49:02 PST
Fixed in <http://trac.webkit.org/projects/webkit/changeset/54784>.