Bug 114314

Summary: FindBanner matches are offset when the WKView has a header or footer
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: Layout and RenderingAssignee: Beth Dakin <bdakin>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, eric, esprehn+autocc, ojan.autocc, sam, simon.fraser, thorton
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch with new names
simon.fraser: review-
Patch simon.fraser: review+

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