Bug 114314 - FindBanner matches are offset when the WKView has a header or footer
Summary: FindBanner matches are offset when the WKView has a header or footer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Beth Dakin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-04-09 15:58 PDT by Beth Dakin
Modified: 2013-04-10 13:56 PDT (History)
8 users (show)

See Also:

Attachments
Patch (10.78 KB, patch)
2013-04-09 16:27 PDT, Beth Dakin 		no flags Details | Formatted Diff | Diff
Patch with new names (11.30 KB, patch)
2013-04-10 13:14 PDT, Beth Dakin 		simon.fraser: review-
Details | Formatted Diff | Diff
Patch (11.30 KB, patch)
2013-04-10 13:25 PDT, Beth Dakin 		simon.fraser: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2013-04-09 15:58:08 PDT 
FindBanner matches don't line up correctly when a WKView has a header or footer. 

<rdar://problem/13522434>
Comment 1 Beth Dakin 2013-04-09 16:27:35 PDT 
Created attachment 197187 [details]
Patch
Comment 2 Beth Dakin 2013-04-10 13:14:22 PDT 
Created attachment 197392 [details]
Patch with new names
Comment 3 Tim Horton 2013-04-10 13:14:57 PDT 
Comment on attachment 197392 [details]
Patch with new names

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

> Source/WebCore/platform/ScrollView.cpp:338
> +IntSize ScrollView::scrollOffsetRealtiveToDocument() const

Relative?
Comment 4 Tim Horton 2013-04-10 13:16:01 PDT 
Comment on attachment 197392 [details]
Patch with new names

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

> Source/WebCore/rendering/RenderLayer.cpp:2384
> +LayoutRect RenderLayer::getRectToExpose(const LayoutRect &visibleRect, LayoutRect &visibleRectRealtiveToDocument, const LayoutRect &exposeRect, const ScrollAlignment& alignX, const ScrollAlignment& alignY)

Relative again.

> Source/WebCore/rendering/RenderLayer.cpp:2424
> +    LayoutUnit intersectHeight = intersection(visibleRectRealtiveToDocument, exposeRectY).height();

And here.
Comment 5 Simon Fraser (smfr) 2013-04-10 13:18:30 PDT 
Comment on attachment 197392 [details]
Patch with new names

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

> Source/WebCore/ChangeLog:17
> +        New function subtracts out the headerHeight() to treat the top of the document at 
> +        (0,0).

Awkward line breaking.

> Source/WebCore/WebCore.exp.in:1227
> +__ZNK7WebCore10ScrollView30scrollOffsetRealtiveToDocumentEv

Typo.
Comment 6 Beth Dakin 2013-04-10 13:25:52 PDT 
Created attachment 197395 [details]
Patch
Comment 7 Simon Fraser (smfr) 2013-04-10 13:38:51 PDT 
Comment on attachment 197395 [details]
Patch

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

> Source/WebCore/ChangeLog:22
> +        Use scrollOffsetRealtiveToDocument() where we were previously making this 

Still misspelledt here.

> Source/WebCore/platform/ScrollView.cpp:753
> +    IntSize offsetRelativeToTotalContents = scrollOffset() + IntSize(0, headerHeight());

offsetIncludingHeader?

> Source/WebCore/rendering/RenderLayer.h:404
> +    LayoutRect getRectToExpose(const LayoutRect& visibleRect, LayoutRect& visibleRectRelativeToDocument, const LayoutRect& exposeRect, const ScrollAlignment& alignX, const ScrollAlignment& alignY);

visibleRectRelativeToDocument should be a const LayoutRect&, no?
Comment 8 Beth Dakin 2013-04-10 13:56:11 PDT 
Thanks Simon!

http://trac.webkit.org/changeset/148137