[webkit-reviews] review granted: [Bug 238985] REGRESSION(r290770): element.scrollIntoViewIfNeeded() scrolls to top even when element is already in viewport : [Attachment 457512] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Rob Buis
<rbuis at igalia.com>'s request for review:
Bug 238985: REGRESSION(r290770): element.scrollIntoViewIfNeeded() scrolls to
top even when element is already in viewport
https://bugs.webkit.org/show_bug.cgi?id=238985

Attachment 457512: Patch

https://bugs.webkit.org/attachment.cgi?id=457512&action=review




--- Comment #7 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 457512
  --> https://bugs.webkit.org/attachment.cgi?id=457512
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=457512&action=review

> Source/WebCore/rendering/RenderLayer.cpp:2659
> +    bool intersects = exposeRect.maxX() >= visibleRect.x() && exposeRect.x()
<= visibleRect.maxX();

call this intersectsInX

> Source/WebCore/rendering/RenderLayer.cpp:2664
> +	   LayoutUnit intersectWidth = intersection(visibleRect,
exposeRectX).width();

Seems like you could compute intersectWidth without making a rect.

> Source/WebCore/rendering/RenderLayer.cpp:-2674
> -	   } else if (intersectWidth > 0) {

Keep the braces because the comment makes it a multi-line clause.

> Source/WebCore/rendering/RenderLayer.cpp:2696
> +    intersects = exposeRect.maxY() >= visibleRect.y() && exposeRect.y() <=
visibleRect.maxY();

bool intersectsInY =

> Source/WebCore/rendering/RenderLayer.cpp:2700
> +	   LayoutRect exposeRectY(visibleRect.x(), exposeRect.y(),
visibleRect.width(), exposeRect.height());

Ditto

> Source/WebCore/rendering/RenderLayer.cpp:2710
> +	   } else if (intersectHeight > 0)

Ditto