Bug 205158

Summary:

Double tap to zoom out doesn't work after upgrading to iOS 13

Product:

WebKit

Reporter:

Rui Chen <ruich>

Component:

UI Events

Assignee:

Antoine Quint <graouts>

Status:

RESOLVED FIXED

Severity:

Major

CC:

dino, graouts, graouts, thorton, webkit-bug-importer, wenson_hsieh

Priority:

Keywords:

InRadar

Version:

Other

Hardware:

iPhone / iPad

OS:

iOS 13

URL:

https://bug-205158-attachments.webkit.org/attachment.cgi?id=385867

See Also:

https://bugs.webkit.org/show_bug.cgi?id=195473

Attachments:

Description	Flags
Testcase	none
double-tap-to-zoom works well on simulator with iOS12.4	none
Patch	none
Patch	none

Rui Chen

Reported 2019-12-12 05:11:31 PST

We use WKWebView as browser in our App. The problem is that double tap to zoom doesn't work when we add a click event listener after upgrading to iOS 13. You could double tap the text on this page(http://www.kappabashi.or.jp/en/) to repro. And below is also a minimum web page can repro. <!DOCTYPE html> <html lang="en"> <head> <meta charset="UTF-8"> <meta name="viewport" content="width=device-width, initial-scale=1.0"> <meta http-equiv="X-UA-Compatible" content="ie=edge"> <title>Document</title> <style> p { width: 50px; height: 200px; } </style> </head> <body> <p class="text">Kappabashi Dougu Street is located in Tokyo's Taito City, along the west side of Asakusa's main entertainment district, and just midway between Ueno and Asakusa.<br> Merchants first began gathering in the Kappabashi area around 1912, selling old tools and a wide range of implements and hardware. Today, one can find almost every kind of restaurant equipment imaginable, including bakery equipment, Japanese, Western, and Chinese tableware, china, laquerware, restaurant equipment, packaging, containers, decorative goods, "fake" food samples, chefs' coats, signs, noren (shop curtains), bamboo wares, baking ingredients, food and beverage ingredients, confectionary wholesalers, Japanese furniture, Western furniture, kitchen equipment, refrigerators and refrigerated showcases, showcases, displays, shop design and construction services, and much more. At 800 meters (nearly a half-mile) in length, and with over 170 shops, this is Japan’s largest shopping street devoted to kitchen implements.</p> <script> document.addEventListener('click', (e) => { }, false); </script> </body> </html>

Attachments
Testcase (1.52 KB, text/html) 2019-12-17 01:55 PST, Antoine Quint	no flags	Details
double-tap-to-zoom works well on simulator with iOS12.4 (1.49 MB, video/quicktime) 2019-12-17 03:33 PST, Rui Chen	no flags	Details
Patch (5.88 KB, patch) 2020-06-12 09:54 PDT, Antoine Quint	no flags	Details Formatted Diff Diff
Patch (6.82 KB, patch) 2020-06-12 09:59 PDT, Antoine Quint	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2019-12-16 13:27:11 PST

<rdar://problem/57982858>

Antoine Quint

Comment 2 2019-12-17 01:17:57 PST

If I visit http://www.kappabashi.or.jp/en/ with an iPhone 8 running iOS 13.3 (17C54) I can double-tap-to-zoom just fine on the text. If I try your proposed reduction I can also double-tap-to-zoom on the narrow text on the left with the same configuration. Please return this with a clearer steps for reproduction and your specific configuration (hardware and software) if you still see the reported issue.

Rui Chen

Comment 3 2019-12-17 01:48:26 PST

Sorry for the unclear description. Double-tap-to-zoom on http://www.kappabashi.or.jp/en/ does work well on Safari. UPDATED REPRO STEPS: 1. Load above static web page I wrote in Safari 2. Double tap the text to zoom-in 3. double-tap-zoom-out doesn't work now. But if remove the `click` event listener, it will work well. I think there are some conflicts between `click` event listener and double-tap-to-zoom. -- HOW WE FOUND THIS: In our app, we inject following codes into WKWebView to listen the click events. After that, double-tap-to-zoom is broken on most of web pages. http://www.kappabashi.or.jp/en/ is a sample which can't zoom in the text after the inject.

Rui Chen

Comment 4 2019-12-17 01:50:13 PST

(In reply to Rui Chen from comment #3) > Sorry for the unclear description. > Double-tap-to-zoom on http://www.kappabashi.or.jp/en/ does work well on > Safari. > > UPDATED REPRO STEPS: > 1. Load above static web page I wrote in Safari > 2. Double tap the text to zoom-in > 3. double-tap-zoom-out doesn't work now. But if remove the `click` event > listener, it will work well. > > I think there are some conflicts between `click` event listener and > double-tap-to-zoom. -- > > > HOW WE FOUND THIS: > In our app, we inject following codes into WKWebView to listen the click > events. > After that, double-tap-to-zoom is broken on most of web pages. > > http://www.kappabashi.or.jp/en/ is a sample which can't zoom in the text > after the inject. I've confirmed this is good on iOS13-

Antoine Quint

Comment 5 2019-12-17 01:52:19 PST

Hi Rui. Are you saying that double-tap-to-zoom-out with the test was working with iOS 13 but not with iOS 13.3?

Antoine Quint

Comment 6 2019-12-17 01:55:05 PST

Created attachment 385867 [details] Testcase

Antoine Quint

Comment 7 2019-12-17 01:58:38 PST

Per my testing, on iOS 12.4 (16G77) the attached test case would not zoom at all on an iPhone 7.

Rui Chen

Comment 8 2019-12-17 02:05:47 PST

(In reply to Antoine Quint from comment #5) > Hi Rui. Are you saying that double-tap-to-zoom-out with the test was working > with iOS 13 but not with iOS 13.3? this bug stable repros on iPhone8 simulator(13.2.2)、iPhone8 (13.2&13.3) but not repro on iPhone7 simulator(10.3.1)

Antoine Quint

Comment 9 2019-12-17 02:44:17 PST

iOS 12.4 (16G77): double-tap-to-zoom has no effect iOS 13.0 (17A577): double-tap-to-zoom has no effect iOS 13.1.3 (17A878): double-tap-to-zoom has no effect iOS 13.2.3 (17B111): double-tap-to-zoom works to zoom in but not to zoom out Based on the testcase, I would argue there is a progression in behavior over iOS 12.4, wouldn't you say? But this could be improved to also allow double tapping to zoom out. Does this match your expectations Rui?

Rui Chen

Comment 10 2019-12-17 03:33:52 PST

Created attachment 385875 [details] double-tap-to-zoom works well on simulator with iOS12.4

Rui Chen

Comment 11 2019-12-17 03:34:40 PST

(In reply to Antoine Quint from comment #9) > iOS 12.4 (16G77): double-tap-to-zoom has no effect > iOS 13.0 (17A577): double-tap-to-zoom has no effect > iOS 13.1.3 (17A878): double-tap-to-zoom has no effect > iOS 13.2.3 (17B111): double-tap-to-zoom works to zoom in but not to zoom out > > Based on the testcase, I would argue there is a progression in behavior over > iOS 12.4, wouldn't you say? But this could be improved to also allow double > tapping to zoom out. > > Does this match your expectations Rui? As my attached video shown, double-tap-to-zoom works to both zoom in and zoom out on iPhone7 simulator with iOS12.4(16G73). And there are only two effects I met: 1. Double-tap-to-zoom works to both zoom in and zoom out 2. Double-tap-to-zoom works to zoom in but not to zoom out

Antoine Quint

Comment 12 2019-12-17 05:10:52 PST

Odd that I got a different behavior with my phone. Regardless, I think this bug is clear now: the issue is that double-tap-to-zoom only works in a single direction.

Antoine Quint

Comment 13 2020-06-12 07:18:47 PDT

When successfully zooming in on a double tap the recognizer is _nonBlockingDoubleTapGestureRecognizer. When failing to double tap out the failing recognizer is _doubleTapGestureRecognizer.

Antoine Quint

Comment 14 2020-06-12 07:32:58 PDT

During the initial interaction the _doubleTapGestureRecognizer is disabled, _doubleTapGestureRecognizer was called with NO. After _didNotHandleTapAsClick is called it is enabled since _setDoubleTapGesturesEnabled was called with YES. When double tapping after zooming in, _setDoubleTapGesturesEnabled is called with NO. Need to track down why.

Antoine Quint

Comment 15 2020-06-12 07:34:20 PDT

If the "click" event listener on the document is removed then we can zoom in and out without worry.

Antoine Quint

Comment 16 2020-06-12 07:39:06 PDT

Double tapping with two fingers zooms out as expected as well.

Antoine Quint

Comment 17 2020-06-12 08:09:57 PDT

We disable double-tap to zoom when double-tapping and already zoomed in because we enter this branch: auto initialScale = [self _initialScaleFactor]; if (std::min(targetScale, initialScale) / std::max(targetScale, initialScale) > fasterTapSignificantZoomThreshold) { RELEASE_LOG(ViewGestures, "Potential tap would not cause a significant zoom. Trigger click. (%p)", self); [self _setDoubleTapGesturesEnabled:NO]; return; }

Antoine Quint

Comment 18 2020-06-12 08:34:08 PDT

I'm not sure why we use the initial scale here rather than the current scale. I think this is an oversight. Will change to use the current scale and write a test, hopefully the bots will indicate whether this code was there with good reason.

Antoine Quint

Comment 19 2020-06-12 09:54:44 PDT

Created attachment 401741 [details] Patch

Wenson Hsieh

Comment 20 2020-06-12 09:56:16 PDT

Comment on attachment 401741 [details] Patch r=mews

Antoine Quint

Comment 21 2020-06-12 09:59:12 PDT

Created attachment 401744 [details] Patch

Antoine Quint

Comment 22 2020-06-12 10:04:09 PDT

This bug was most likely introduced by r242757, the commit for bug 195473.