[webkit-reviews] review denied: [Bug 53239] Wrapping text overlaps floated element(s) when there are 2 or more adjacent floated elements : [Attachment 99832] Patch

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has denied Jacky Jiang
<zkjiang008 at gmail.com>'s request for review:
Bug 53239: Wrapping text overlaps floated element(s) when there are 2 or more
adjacent floated elements
https://bugs.webkit.org/show_bug.cgi?id=53239

Attachment 99832: Patch
https://bugs.webkit.org/attachment.cgi?id=99832&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=99832&action=review

It's hard to judge about this patch, without a more detailed description - can
you please come up with more details? :-)

> LayoutTests/ChangeLog:10
> +	   Reviewed by NOBODY (OOPS!).

This line belongs before your description.

> LayoutTests/ChangeLog:15
> +	   * fast/block/float/text-in-paragraph-overlapping-floats.html: Added.

> +	   *
platform/qt/fast/block/float/text-in-paragraph-overlapping-floats-expected.png:
Added.
> +	   *
platform/qt/fast/block/float/text-in-paragraph-overlapping-floats-expected.txt:
Added.
> +	   * platform/qt/css1/formatting_model/floating_elements-expected.txt

You need to generate results with a mac and include them as well.

> Source/WebCore/ChangeLog:8
> +	   Text in paragraph overlapping adjacent floats.
> +	   Shrink logical offset for line to avoid adjacent floats when the
tops of floats are just
> +	   between the top and bottom of the line.

This should be more verbose. Tell us what's the problem first and then how you
fixed it, currently you're just describing what you've changed.
I'm not understanding it at present.

> Source/WebCore/ChangeLog:10
> +	   Reviewed by NOBODY (OOPS!).

This line belongs before your description.