Bug 135480

Summary: Inspector highlights clipped at the bottom on the page in WK1 views with contentInsets
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, benjamin, commit-queue, esprehn+autocc, glenn, kondapallykalyan, sam, simon.fraser, thorton
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch with a new name simon.fraser: review+

Description Beth Dakin 2014-07-31 16:37:27 PDT
Inspector highlights are clipped at the bottom on the page in WK1 views with contentInsets. This is the WK1 versions of this bug: https://bugs.webkit.org/show_bug.cgi?id=133818 It is reproducible in MiniBrowser on Yosemite at the bottom of any page.

<rdar://problem/17850323>
Comment 1 Beth Dakin 2014-07-31 16:57:20 PDT
Created attachment 235860 [details]
Patch
Comment 2 Beth Dakin 2014-08-01 12:15:36 PDT
Created attachment 235899 [details]
Patch with a new name

Simon and I discussed the new name on IRC, and we like this better.
Comment 3 Simon Fraser (smfr) 2014-08-01 14:33:58 PDT
Comment on attachment 235899 [details]
Patch with a new name

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

Is this testable?

> Source/WebCore/platform/ScrollView.h:485
>      IntRect platformVisibleContentRect(bool includeScrollbars) const;
>      IntSize platformVisibleContentSize(bool includeScrollbars) const;
> +    IntRect platformVisibleContentRectIncludingObscuredArea(bool includeScrollbars) const;
> +    IntSize platformVisibleContentSizeIncludingObscuredArea(bool includeScrollbars) const;

It almost feels like the "including obscured area" could be an enum param to platformVisibleContentRect/Size.
Comment 4 Beth Dakin 2014-08-01 15:41:18 PDT
Thanks Simon! Joe and I are still thinking about a test case…we haven't come up with one yet, but it would be very valuable. In terms of the enum parameter, I do think that is a really good idea, and I think we should consider it when we clean up these functions, which I really really want to do.

http://trac.webkit.org/changeset/171951
Comment 5 Beth Dakin 2014-08-01 15:49:13 PDT
Oops, http://trac.webkit.org/changeset/171952 is needed too! It's critical, in fact! I accidentally had that part of the change reverted from when I was trying to see if the layout tests I was trying to make actually triggered a difference from this change.