Bug 170981 - Pinch-zoom should be invisible to the application by default
Summary: Pinch-zoom should be invisible to the application by default
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari 10
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on: 171113 174362 172018 172019 174734
Blocks:
  Show dependency treegraph
 
Reported: 2017-04-19 00:50 PDT by Rick Byers
Modified: 2019-01-29 20:49 PST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Rick Byers 2017-04-19 00:50:20 PDT
Developers typically expect APIs like MouseEvent.clientX/Y and getBoundingClientRect to use the same co-ordinate system as position:fixed (what we've been calling "layout viewport co-ordinates").  In Safari's new visual viewport model these APIs return visual co-ordinates instead, resulting in some site bugs under pinch-zoom.  As a simple example see https://rbyers.github.io/inputCoords.html - when pinch-zooming in the red box is wrong.

In addition, some sites don't expect scroll offsets to change when panning the pinch zoom viewport, or that changing the scroll position will reset the position of the pinch zoom viewport.  For example on apple.com if you zoom in and click the search button in the top bar the viewport doesn't get centered correctly on the search field because there's a window.scrollTo(0,0) call.

Here's some example sites that are broken in Safari today:
https://docs.google.com/document/d/1jxWEzwOT2XgSsg5k2a_3D7PR0bXnB4bL9rATW-Z6egY/edit

Talking with smfr@ he agrees with the principle that pinch-zoom should be completely transparent to the application by default.  Developers generally are not thinking about the impact of pinch-zoom, and pinch-zoom is primarily a UA/accessibility feature.  He says he'd like to change WebKit to make all these APIs use layout viewport co-ordinates instead of visual viewport ones (i.e. to match Chrome's behavior with the chrome://flags/#inert-visual-viewport flag enabled - see https://crbug.com/489206)
Comment 1 Rick Byers 2017-04-19 01:04:53 PDT
See bug 170982 for a possible way to explicitly expose pinch-zoom to applications if desired.
Comment 2 Rick Byers 2017-04-19 01:06:15 PDT
An old list of APIs and which co-ordinate systems they currently use in blink:
https://docs.google.com/spreadsheets/d/11DfDDa-s1hePVwBn3PZidlPJZ9ahhkJ44dyuMiOQtrc/edit#gid=0

We intend to change all the ones marked 'visual viewport' to 
'layout viewport'.
Comment 3 Radar WebKit Bug Importer 2017-04-19 01:08:12 PDT
<rdar://problem/31702298>
Comment 4 Simon Fraser (smfr) 2017-04-19 01:14:50 PDT
This seems like a reasonable approach.
Comment 5 David Bokan 2017-04-24 07:44:19 PDT
Would it make sense to only make this change at the same time as shipping the viewport API (https://github.com/WICG/ViewportAPI)? There are some reasonable though niche use cases that lose some abilities when pinch-zoom is made invisible. We got some non-trivial push-back from web devs in http://crbug.com/571297 when we did this.

The presence of the API would also be a convenient way to feature-detect the change, which is a request we've gotten: https://github.com/WICG/ViewportAPI/issues/24. Otherwise, the page has no way to  know how window.innerWidth et al. will behave.
Comment 6 Simon Fraser (smfr) 2017-07-19 19:22:00 PDT
I'm seeing issues with baidu.com, which is using jQuery's offset() function, and using the result to do a scrollTo().

offset().top is basically elementgetBoundingClientRect().top + window.pageYOffset, so using the result for scrollTo() doesn't scroll that element to the top if the top of the layout viewport is not at scrollY.

This implies that scroll positions should be the origin of the layout viewport rect?
Comment 7 David Bokan 2017-07-20 05:36:23 PDT
Right, that's a common pattern I've seen is to assume that "position in document" = "getBoundingClientRect + window.scrollX/Y".

This doesn't work in Safari (and Chrome until M61) since getBoundingClientRect returns the position relative to the layout viewport but window.scrollX/Y is the position of the visual viewport. In M61, we've made window.scrollX/Y return the offset of the layout viewport.

Note: we've also made window.scrollTo scroll only the layout viewport.
Comment 8 Simon Fraser (smfr) 2017-07-20 09:25:20 PDT
(In reply to David Bokan from comment #7)
> Right, that's a common pattern I've seen is to assume that "position in
> document" = "getBoundingClientRect + window.scrollX/Y".
> 
> This doesn't work in Safari (and Chrome until M61) since
> getBoundingClientRect returns the position relative to the layout viewport
> but window.scrollX/Y is the position of the visual viewport. In M61, we've
> made window.scrollX/Y return the offset of the layout viewport.
> 
> Note: we've also made window.scrollTo scroll only the layout viewport.

Thanks David. Could you update https://docs.google.com/spreadsheets/d/11DfDDa-s1hePVwBn3PZidlPJZ9ahhkJ44dyuMiOQtrc/edit#gid=0 with what Chrome is doing now?

Changing scrollX/Y to use the layout viewport is a larger change than I can do now. I wonder if we should get a jQuery fix? I think it would work to do something like element.getBoundingClientRct().y + documentElement.getBoundingClientRct() - window.pageYOffset?
Comment 9 David Bokan 2017-07-20 11:59:10 PDT
> Thanks David. Could you update
> https://docs.google.com/spreadsheets/d/11DfDDa-
> s1hePVwBn3PZidlPJZ9ahhkJ44dyuMiOQtrc/edit#gid=0 with what Chrome is doing
> now?

Done. Chrome should be all-layout starting in M61 (I actually found a bug going through the list - Element.scrollTo and friends is still visual) but I should have that fixed in time).

> 
> Changing scrollX/Y to use the layout viewport is a larger change than I can
> do now. I wonder if we should get a jQuery fix? I think it would work to do
> something like element.getBoundingClientRct().y +
> documentElement.getBoundingClientRct() - window.pageYOffset?

Yeah, I would expect that should work