Bug 135480 - Inspector highlights clipped at the bottom on the page in WK1 views with contentInsets
Summary: Inspector highlights clipped at the bottom on the page in WK1 views with cont...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Keywords: InRadar
Depends on:
Reported: 2014-07-31 16:37 PDT by Beth Dakin
Modified: 2014-08-01 15:49 PDT (History)
9 users (show)

See Also:

Patch (12.78 KB, patch)
2014-07-31 16:57 PDT, Beth Dakin
no flags Details | Formatted Diff | Diff
Patch with a new name (12.89 KB, patch)
2014-08-01 12:15 PDT, Beth Dakin
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 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.

Comment 1 Beth Dakin 2014-07-31 16:57:20 PDT
Created attachment 235860 [details]
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.

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.