[webkit-reviews] review granted: [Bug 34923] Incorrect client rects for blocks the span multiple columns and their descendants : [Attachment 48732] Allow offsetFromContainer() to return the right offset for a given point in the multi-column case, and pass it the point of interest

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted mitz at webkit.org's
request for review:
Bug 34923: Incorrect client rects for blocks the span multiple columns and
their descendants
https://bugs.webkit.org/show_bug.cgi?id=34923

Attachment 48732: Allow offsetFromContainer() to return the right offset for a
given point in the multi-column case, and pass it the point of interest
https://bugs.webkit.org/attachment.cgi?id=48732&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog (revision 54757)
> +++ WebCore/ChangeLog (working copy)
> @@ -1,3 +1,63 @@
> +2010-02-14  Dan Bernstein  <mitz at 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.