Bug 190570

Summary:

Changing view scale should zoom to initial scale if the page is already at initial scale

Product:

WebKit

Reporter:

Wenson Hsieh <wenson_hsieh>

Component:

WebKit Misc.

Assignee:

Wenson Hsieh <wenson_hsieh>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bdakin, commit-queue, simon.fraser, thorton, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Add a layout test description	thorton: review+
Patch for landing	none

Wenson Hsieh

Reported 2018-10-14 20:52:12 PDT

Changing zoom levels should zoom to initial scale if the page is already at initial scale

Attachments
Patch (13.66 KB, patch) 2018-10-15 08:19 PDT, Wenson Hsieh	no flags	Details Formatted Diff Diff
Add a layout test description (14.94 KB, patch) 2018-10-15 08:39 PDT, Wenson Hsieh	thorton: review+	Details Formatted Diff Diff
Patch for landing (15.92 KB, patch) 2018-10-15 09:59 PDT, Wenson Hsieh	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2018-10-14 20:52:49 PDT

<rdar://problem/45261877>

Wenson Hsieh

Comment 2 2018-10-15 08:19:15 PDT

Created attachment 352320 [details] Patch

Wenson Hsieh

Comment 3 2018-10-15 08:39:29 PDT

Created attachment 352325 [details] Add a layout test description

Tim Horton

Comment 4 2018-10-15 09:29:50 PDT

Comment on attachment 352325 [details] Add a layout test description View in context: https://bugs.webkit.org/attachment.cgi?id=352325&action=review > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2526 > + if (m_viewportConfiguration.layoutSizeScaleFactor() != scaleFactor && std::abs(m_viewportConfiguration.initialScale() - pageScaleFactor()) <= 0.025) I think you might want areEssentiallyEqual for float compare. At least, Ben used it in a few places because we do so much math with these scales that they’d end up “equal” but only in that sense, and things would break. Don’t know if it’s a concern in this path.

Wenson Hsieh

Comment 5 2018-10-15 09:56:48 PDT

Comment on attachment 352325 [details] Add a layout test description View in context: https://bugs.webkit.org/attachment.cgi?id=352325&action=review >> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2526 >> + if (m_viewportConfiguration.layoutSizeScaleFactor() != scaleFactor && std::abs(m_viewportConfiguration.initialScale() - pageScaleFactor()) <= 0.025) > > I think you might want areEssentiallyEqual for float compare. At least, Ben used it in a few places because we do so much math with these scales that they’d end up “equal” but only in that sense, and things would break. Don’t know if it’s a concern in this path. I originally added this fudge factor so that if a user manually pinched to something close to initial scale, we'd still zoom to the new initial scale upon zoom level change. On second thought, I don't think this behavior is all that valuable — I changed this to just use areEssentiallyEqualAsFloat(), like all of the other page scale checks in this file.

Wenson Hsieh

Comment 6 2018-10-15 09:59:12 PDT

Created attachment 352335 [details] Patch for landing

WebKit Commit Bot

Comment 7 2018-10-15 10:36:39 PDT

Comment on attachment 352335 [details] Patch for landing Clearing flags on attachment: 352335 Committed r237127: <https://trac.webkit.org/changeset/237127>

Note You need to log in before you can comment on or make changes to this bug.