Bug 140632 - [iOS][WK2] Redraw find-in-page indicator on rotation
Summary: [iOS][WK2] Redraw find-in-page indicator on rotation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Hock
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-19 12:03 PST by Martin Hock
Modified: 2015-01-20 14:21 PST (History)
2 users (show)

See Also:


Attachments
patch (3.42 KB, patch)
2015-01-19 12:11 PST, Martin Hock
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>