Bug 140632

Summary: [iOS][WK2] Redraw find-in-page indicator on rotation
Product: WebKit Reporter: Martin Hock <mhock>
Component: WebKit2Assignee: Martin Hock <mhock>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, thorton
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch thorton: review+

Description Martin Hock 2015-01-19 12:03:24 PST
Redraw find-in-page indicator on rotation
Comment 1 Martin Hock 2015-01-19 12:11:04 PST
Created attachment 244915 [details]
patch
Comment 2 Tim Horton 2015-01-20 13:44:05 PST
Comment on attachment 244915 [details]
patch

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

> Source/WebKit2/WebProcess/WebPage/FindController.cpp:374
> +    if (m_isShowingFindIndicator) {

this should be an early return.

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3481
> +    m_findController.redraw();

Might this be helpful on Mac too? Does it do the right thing there? (I'm guessing not, so this is probably best for now)

> Source/WebKit2/WebProcess/WebPage/ios/FindControllerIOS.mm:99
> +        m_webPage->zoomToRect(matchRect, m_webPage->minimumPageScaleFactor(), std::min(m_webPage->maximumPageScaleFactor(), maximumFindIndicatorZoom));

Why this change? If the highlighted text moves out of view after a layout, wouldn't we want to zoom to it?
Comment 3 Martin Hock 2015-01-20 13:54:07 PST
(In reply to comment #2)
> > Source/WebKit2/WebProcess/WebPage/FindController.cpp:374
> > +    if (m_isShowingFindIndicator) {
> 
> this should be an early return.

Will do.

> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3481
> > +    m_findController.redraw();
> 
> Might this be helpful on Mac too? Does it do the right thing there? (I'm
> guessing not, so this is probably best for now)

The changes don't seem to be necessary on Mac.

> > Source/WebKit2/WebProcess/WebPage/ios/FindControllerIOS.mm:99
> > +        m_webPage->zoomToRect(matchRect, m_webPage->minimumPageScaleFactor(), std::min(m_webPage->maximumPageScaleFactor(), maximumFindIndicatorZoom));
> 
> Why this change? If the highlighted text moves out of view after a layout,
> wouldn't we want to zoom to it?

I don't think we'd like the view to move constantly if text is animated. Furthermore, the user panning could trigger this code and we wouldn't like that to move the view around, either.
Comment 4 Martin Hock 2015-01-20 14:21:22 PST
Committed r178755: <http://trac.webkit.org/changeset/178755>