Skip to content

Commit b5bed29

Browse files
whsiehwhsieh
committed
REGRESSION (r271660): Tap highlight no longer shows when tapping clickable elements without touch event listeners
https://bugs.webkit.org/show_bug.cgi?id=224385 <rdar://problem/76462370> Reviewed by Tim Horton. Source/WebKit: In r271660, I added a call to `-finishInteraction` when resetting the synthetic tap gesture to fix a bug where it was possible for the tap highlight to remain indefinitely when tapping on a clickable element with a touchend event listener. This was because the touch end deferring gesture defers `-_singleTapDidReset:` until after `-_didGetTapHighlightForRequest:…nodeHasBuiltInClickHandling:`, where we receive the tap highlight information. ``` 2021-04-09 13:05:27.141097-0700 -[WKContentView(WKInteraction) _singleTapIdentified:] 2021-04-09 13:05:27.148678-0700 -[WKContentView(WKInteraction) _didGetTapHighlightForRequest:…nodeHasBuiltInClickHandling:] 2021-04-09 13:05:27.162525-0700 -[WKContentView(WKInteraction) _singleTapRecognized:] 2021-04-09 13:05:27.162675-0700 ↳ -[WKContentView(WKInteraction) _showTapHighlight] 2021-04-09 13:05:27.163250-0700 -[WKContentView(WKInteraction) _singleTapDidReset:] 2021-04-09 13:05:51.849481-0700 ↳ -[WKContentView(WKInteraction) _finishInteraction] ``` However, in the case where there is no touchend event listener and when fast-click is active, we reset the tap gesture before receiving the tap highlight information: ``` 2021-04-09 13:05:51.836638-0700 -[WKContentView(WKInteraction) _singleTapIdentified:] 2021-04-09 13:05:51.846152-0700 -[WKContentView(WKInteraction) _singleTapRecognized:] 2021-04-09 13:05:51.847196-0700 -[WKContentView(WKInteraction) _singleTapDidReset:] 2021-04-09 13:05:51.848563-0700 -[WKContentView(WKInteraction) _didGetTapHighlightForRequest:…nodeHasBuiltInClickHandling:] 2021-04-09 13:05:51.848851-0700 ↳ -[WKContentView(WKInteraction) _showTapHighlight] 2021-04-09 13:05:51.849481-0700 ↳ -[WKContentView(WKInteraction) _finishInteraction] ``` Critically, this means that calling `-_finishInteraction` in `-_singleTapDidReset:` prematurely flagged the tap highlight request as complete (by setting `_isTapHighlightIDValid` to `NO`) in the case where we aren't deferring gestures, which caused us to avoid showing the tap highlight at all when we eventually receive the tap highlight information. To fix this, only fade out the tap highlight view in `-_singleTapDidReset:` if the tap highlight request has already finished (i.e. `_isTapHighlightIDValid` has been set to `NO`). Additionally, split logic for fading out the highlight view into a separate method, and add a `BOOL` flag to make the fading idempotent. Test: fast/events/touch/ios/tap-highlight-during-synthetic-click.html * UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h: * UIProcess/API/ios/WKWebViewTestingIOS.mm: (-[WKWebView _tapHighlightViewRect]): * UIProcess/ios/WKContentViewInteraction.h: * UIProcess/ios/WKContentViewInteraction.mm: (-[WKContentView tapHighlightViewRect]): Also add a testing-only method to report the frame of the tap highlight view. Note that this only attempts to return the current `frame` of the tap highlight view instead of converting the frame to the coordinate system of the content view (as other similar testing hooks do), since the tap highlight view only exists in the view hierarchy for a brief duration. (-[WKContentView _finishInteraction]): (-[WKContentView _fadeTapHighlightViewIfNeeded]): (-[WKContentView _singleTapDidReset:]): Tools: Add plumbing to expose the frame of the tap highlight view via `UIScriptController`. * TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl: * TestRunnerShared/UIScriptContext/UIScriptController.h: (WTR::UIScriptController::tapHighlightViewRect const): * WebKitTestRunner/ios/UIScriptControllerIOS.h: * WebKitTestRunner/ios/UIScriptControllerIOS.mm: (WTR::UIScriptControllerIOS::tapHighlightViewRect const): LayoutTests: Add a new layout test that exercises 3 scenarios, using the new testing SPI: 1. Tapping on a `button` that prevents the "touchend" event (and therefore prevents clicking) should cause the tap highlight to not show up. 2. Tapping on a `button` that has a "touchend" event listener and does not prevent default should cause the tap highlight to show up. 3. Tapping on a `button` with no event listeners should cause the tap highlight to show up. * fast/events/touch/ios/tap-highlight-during-synthetic-click-expected.txt: Added. * fast/events/touch/ios/tap-highlight-during-synthetic-click.html: Added. * resources/ui-helper.js: (window.UIHelper.tapHighlightViewRect): Canonical link: https://commits.webkit.org/236362@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275791 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent c4fe229 commit b5bed29

14 files changed

Lines changed: 256 additions & 8 deletions

File tree

‎LayoutTests/ChangeLog‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,24 @@
1+
2021-04-09 Wenson Hsieh <wenson_hsieh@apple.com>
2+
3+
REGRESSION (r271660): Tap highlight no longer shows when tapping clickable elements without touch event listeners
4+
https://bugs.webkit.org/show_bug.cgi?id=224385
5+
<rdar://problem/76462370>
6+
7+
Reviewed by Tim Horton.
8+
9+
Add a new layout test that exercises 3 scenarios, using the new testing SPI:
10+
11+
1. Tapping on a `button` that prevents the "touchend" event (and therefore prevents clicking) should cause
12+
the tap highlight to not show up.
13+
2. Tapping on a `button` that has a "touchend" event listener and does not prevent default should cause the tap
14+
highlight to show up.
15+
3. Tapping on a `button` with no event listeners should cause the tap highlight to show up.
16+
17+
* fast/events/touch/ios/tap-highlight-during-synthetic-click-expected.txt: Added.
18+
* fast/events/touch/ios/tap-highlight-during-synthetic-click.html: Added.
19+
* resources/ui-helper.js:
20+
(window.UIHelper.tapHighlightViewRect):
21+
122
2021-04-09 Alexey Shvayka <shvaikalesh@gmail.com>
223

324
Remove className() and toStringName() from the method table

‎LayoutTests/fast/events/touch/ios/tap-highlight-during-synthetic-click-expected.txt‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
Target 1
2+
Target 2
3+
Target 3
4+
This test verifies that tap highlights are shown when tapping on clickable elements. To run the test manually, tap on the three target buttons; a tap highlight should *not* appear over the first target, but should appear over targets 2 and 3.
5+
6+
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
7+
8+
9+
PASS initialTapHighlightRect.left is lastKnownHighlightRect.left
10+
PASS initialTapHighlightRect.top is lastKnownHighlightRect.top
11+
PASS initialTapHighlightRect.width is lastKnownHighlightRect.width
12+
PASS initialTapHighlightRect.height is lastKnownHighlightRect.height
13+
PASS Tap highlight shown after tapping on target 2.
14+
PASS Tap highlight shown after tapping on target 3.
15+
PASS successfullyParsed is true
16+
17+
TEST COMPLETE
18+

‎LayoutTests/fast/events/touch/ios/tap-highlight-during-synthetic-click.html‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
2+
<html>
3+
<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
4+
<head>
5+
<script src="../../../../resources/js-test.js"></script>
6+
<script src="../../../../resources/ui-helper.js"></script>
7+
<script>
8+
jsTestIsAsync = true;
9+
10+
async function waitForTapHighlightRectToChange(fromRect)
11+
{
12+
let rect;
13+
do {
14+
rect = await UIHelper.tapHighlightViewRect();
15+
} while (rect.top == fromRect.top && rect.left == fromRect.left && rect.width == fromRect.width && rect.height == fromRect.height);
16+
return rect;
17+
}
18+
19+
addEventListener("load", async () => {
20+
const target1 = document.getElementById("target1");
21+
const target2 = document.getElementById("target2");
22+
const target3 = document.getElementById("target3");
23+
24+
target1.addEventListener("touchend", event => {
25+
event.preventDefault();
26+
});
27+
target2.addEventListener("touchend", event => { });
28+
29+
description("This test verifies that tap highlights are shown when tapping on clickable elements. To run the test manually, tap on the three target buttons; a tap highlight should *not* appear over the first target, but should appear over targets 2 and 3.")
30+
31+
if (!window.testRunner)
32+
return;
33+
34+
// Since the touch event was prevented, no tap highlight rect should be shown.
35+
initialTapHighlightRect = await UIHelper.tapHighlightViewRect();
36+
await UIHelper.activateElement(target1);
37+
await UIHelper.waitForDoubleTapDelay();
38+
lastKnownHighlightRect = await UIHelper.tapHighlightViewRect();
39+
shouldBe("initialTapHighlightRect.left", "lastKnownHighlightRect.left");
40+
shouldBe("initialTapHighlightRect.top", "lastKnownHighlightRect.top");
41+
shouldBe("initialTapHighlightRect.width", "lastKnownHighlightRect.width");
42+
shouldBe("initialTapHighlightRect.height", "lastKnownHighlightRect.height");
43+
44+
await UIHelper.activateElement(target2);
45+
await UIHelper.waitForDoubleTapDelay();
46+
lastKnownHighlightRect = await waitForTapHighlightRectToChange(lastKnownHighlightRect);
47+
testPassed("Tap highlight shown after tapping on target 2.");
48+
49+
await UIHelper.activateElement(target3);
50+
await UIHelper.waitForDoubleTapDelay();
51+
lastKnownHighlightRect = await waitForTapHighlightRectToChange(lastKnownHighlightRect);
52+
testPassed("Tap highlight shown after tapping on target 3.");
53+
54+
finishJSTest();
55+
});
56+
</script>
57+
<style>
58+
#target1 {
59+
background-color: orange;
60+
}
61+
62+
#target2 {
63+
background-color: purple;
64+
}
65+
66+
#target3 {
67+
background-color: tomato;
68+
}
69+
70+
.target {
71+
width: 150px;
72+
height: 50px;
73+
margin: 3em;
74+
display: block;
75+
}
76+
</style>
77+
</head>
78+
<body>
79+
<button id="target1" class="target">Target 1</button>
80+
<button id="target2" class="target">Target 2</button>
81+
<button id="target3" class="target">Target 3</button>
82+
<div id="console"></div>
83+
</body>
84+
</html>

‎LayoutTests/resources/ui-helper.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -819,6 +819,15 @@ window.UIHelper = class UIHelper {
819819
});
820820
}
821821

822+
static tapHighlightViewRect()
823+
{
824+
return new Promise(resolve => {
825+
testRunner.runUIScript("JSON.stringify(uiController.tapHighlightViewRect)", jsonString => {
826+
resolve(JSON.parse(jsonString));
827+
});
828+
});
829+
}
830+
822831
static replaceTextAtRange(text, location, length) {
823832
return new Promise(resolve => {
824833
testRunner.runUIScript(`(() => {

‎Source/WebKit/ChangeLog‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,65 @@
1+
2021-04-09 Wenson Hsieh <wenson_hsieh@apple.com>
2+
3+
REGRESSION (r271660): Tap highlight no longer shows when tapping clickable elements without touch event listeners
4+
https://bugs.webkit.org/show_bug.cgi?id=224385
5+
<rdar://problem/76462370>
6+
7+
Reviewed by Tim Horton.
8+
9+
In r271660, I added a call to `-finishInteraction` when resetting the synthetic tap gesture to fix a bug where
10+
it was possible for the tap highlight to remain indefinitely when tapping on a clickable element with a touchend
11+
event listener. This was because the touch end deferring gesture defers `-_singleTapDidReset:` until after
12+
`-_didGetTapHighlightForRequest:…nodeHasBuiltInClickHandling:`, where we receive the tap highlight information.
13+
14+
```
15+
2021-04-09 13:05:27.141097-0700 -[WKContentView(WKInteraction) _singleTapIdentified:]
16+
2021-04-09 13:05:27.148678-0700 -[WKContentView(WKInteraction) _didGetTapHighlightForRequest:…nodeHasBuiltInClickHandling:]
17+
2021-04-09 13:05:27.162525-0700 -[WKContentView(WKInteraction) _singleTapRecognized:]
18+
2021-04-09 13:05:27.162675-0700 ↳ -[WKContentView(WKInteraction) _showTapHighlight]
19+
2021-04-09 13:05:27.163250-0700 -[WKContentView(WKInteraction) _singleTapDidReset:]
20+
2021-04-09 13:05:51.849481-0700 ↳ -[WKContentView(WKInteraction) _finishInteraction]
21+
```
22+
23+
However, in the case where there is no touchend event listener and when fast-click is active, we reset the tap
24+
gesture before receiving the tap highlight information:
25+
26+
```
27+
2021-04-09 13:05:51.836638-0700 -[WKContentView(WKInteraction) _singleTapIdentified:]
28+
2021-04-09 13:05:51.846152-0700 -[WKContentView(WKInteraction) _singleTapRecognized:]
29+
2021-04-09 13:05:51.847196-0700 -[WKContentView(WKInteraction) _singleTapDidReset:]
30+
2021-04-09 13:05:51.848563-0700 -[WKContentView(WKInteraction) _didGetTapHighlightForRequest:…nodeHasBuiltInClickHandling:]
31+
2021-04-09 13:05:51.848851-0700 ↳ -[WKContentView(WKInteraction) _showTapHighlight]
32+
2021-04-09 13:05:51.849481-0700 ↳ -[WKContentView(WKInteraction) _finishInteraction]
33+
34+
```
35+
36+
Critically, this means that calling `-_finishInteraction` in `-_singleTapDidReset:` prematurely flagged the
37+
tap highlight request as complete (by setting `_isTapHighlightIDValid` to `NO`) in the case where we aren't
38+
deferring gestures, which caused us to avoid showing the tap highlight at all when we eventually receive the tap
39+
highlight information.
40+
41+
To fix this, only fade out the tap highlight view in `-_singleTapDidReset:` if the tap highlight request has
42+
already finished (i.e. `_isTapHighlightIDValid` has been set to `NO`). Additionally, split logic for fading out
43+
the highlight view into a separate method, and add a `BOOL` flag to make the fading idempotent.
44+
45+
Test: fast/events/touch/ios/tap-highlight-during-synthetic-click.html
46+
47+
* UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h:
48+
* UIProcess/API/ios/WKWebViewTestingIOS.mm:
49+
(-[WKWebView _tapHighlightViewRect]):
50+
* UIProcess/ios/WKContentViewInteraction.h:
51+
* UIProcess/ios/WKContentViewInteraction.mm:
52+
(-[WKContentView tapHighlightViewRect]):
53+
54+
Also add a testing-only method to report the frame of the tap highlight view. Note that this only attempts to
55+
return the current `frame` of the tap highlight view instead of converting the frame to the coordinate system of
56+
the content view (as other similar testing hooks do), since the tap highlight view only exists in the view
57+
hierarchy for a brief duration.
58+
59+
(-[WKContentView _finishInteraction]):
60+
(-[WKContentView _fadeTapHighlightViewIfNeeded]):
61+
(-[WKContentView _singleTapDidReset:]):
62+
163
2021-04-09 Brent Fulgham <bfulgham@apple.com>
264

365
[iOS] Correct process-info rules to restrict to self

‎Source/WebKit/UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
@property (nonatomic, readonly) NSNumber *_stableStateOverride;
4545
@property (nonatomic, readonly) CGRect _dragCaretRect;
4646
@property (nonatomic, readonly, getter=_isAnimatingDragCancel) BOOL _animatingDragCancel;
47+
@property (nonatomic, readonly) CGRect _tapHighlightViewRect;
4748

4849
- (void)keyboardAccessoryBarNext;
4950
- (void)keyboardAccessoryBarPrevious;

‎Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,11 @@ - (BOOL)_isAnimatingDragCancel
331331
#endif
332332
}
333333

334+
- (CGRect)_tapHighlightViewRect
335+
{
336+
return [_contentView tapHighlightViewRect];
337+
}
338+
334339
- (void)_simulateElementAction:(_WKElementActionType)actionType atLocation:(CGPoint)location
335340
{
336341
[_contentView _simulateElementAction:actionType atLocation:location];

‎Source/WebKit/UIProcess/ios/WKContentViewInteraction.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,7 @@ enum class ProceedWithImageExtraction : bool {
390390
BOOL _showingTextStyleOptions;
391391
BOOL _hasValidPositionInformation;
392392
BOOL _isTapHighlightIDValid;
393+
BOOL _isTapHighlightFading;
393394
BOOL _potentialTapInProgress;
394395
BOOL _isDoubleTapPending;
395396
BOOL _longPressCanClick;
@@ -521,6 +522,7 @@ enum class ProceedWithImageExtraction : bool {
521522
@property (nonatomic, readonly) UIWebTouchEventsGestureRecognizer *touchEventGestureRecognizer;
522523
@property (nonatomic, readonly) NSArray<WKDeferringGestureRecognizer *> *deferringGestures;
523524
@property (nonatomic, readonly) WebKit::GestureRecognizerConsistencyEnforcer& gestureRecognizerConsistencyEnforcer;
525+
@property (nonatomic, readonly) CGRect tapHighlightViewRect;
524526

525527
#if ENABLE(DATALIST_ELEMENT)
526528
@property (nonatomic, strong) UIView <WKFormControl> *dataListTextSuggestionsInputView;

‎Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1909,6 +1909,11 @@ - (void)_updateTapHighlight
19091909
]];
19101910
}
19111911

1912+
- (CGRect)tapHighlightViewRect
1913+
{
1914+
return [_highlightView frame];
1915+
}
1916+
19121917
- (void)_showTapHighlight
19131918
{
19141919
auto shouldPaintTapHighlight = [&](const WebCore::FloatRect& rect) {
@@ -2674,15 +2679,25 @@ - (void)_cancelInteraction
26742679
- (void)_finishInteraction
26752680
{
26762681
_isTapHighlightIDValid = NO;
2682+
[self _fadeTapHighlightViewIfNeeded];
2683+
}
2684+
2685+
- (void)_fadeTapHighlightViewIfNeeded
2686+
{
2687+
if (![_highlightView superview] || _isTapHighlightFading)
2688+
return;
2689+
2690+
_isTapHighlightFading = YES;
26772691
CGFloat tapHighlightFadeDuration = _showDebugTapHighlightsForFastClicking ? 0.25 : 0.1;
26782692
[UIView animateWithDuration:tapHighlightFadeDuration
2679-
animations:^{
2680-
[_highlightView layer].opacity = 0;
2681-
}
2682-
completion:^(BOOL finished){
2683-
if (finished)
2684-
[_highlightView removeFromSuperview];
2685-
}];
2693+
animations:^{
2694+
[_highlightView layer].opacity = 0;
2695+
}
2696+
completion:^(BOOL finished) {
2697+
if (finished)
2698+
[_highlightView removeFromSuperview];
2699+
_isTapHighlightFading = NO;
2700+
}];
26862701
}
26872702

26882703
- (BOOL)canShowNonEmptySelectionView
@@ -2953,7 +2968,8 @@ - (void)_singleTapDidReset:(UITapGestureRecognizer *)gestureRecognizer
29532968
for (const auto& action : actionsToPerform)
29542969
action();
29552970

2956-
[self _finishInteraction];
2971+
if (!_isTapHighlightIDValid)
2972+
[self _fadeTapHighlightViewIfNeeded];
29572973
}
29582974

29592975
- (void)_doubleTapDidFail:(UITapGestureRecognizer *)gestureRecognizer

‎Tools/ChangeLog‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,20 @@
1+
2021-04-09 Wenson Hsieh <wenson_hsieh@apple.com>
2+
3+
REGRESSION (r271660): Tap highlight no longer shows when tapping clickable elements without touch event listeners
4+
https://bugs.webkit.org/show_bug.cgi?id=224385
5+
<rdar://problem/76462370>
6+
7+
Reviewed by Tim Horton.
8+
9+
Add plumbing to expose the frame of the tap highlight view via `UIScriptController`.
10+
11+
* TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
12+
* TestRunnerShared/UIScriptContext/UIScriptController.h:
13+
(WTR::UIScriptController::tapHighlightViewRect const):
14+
* WebKitTestRunner/ios/UIScriptControllerIOS.h:
15+
* WebKitTestRunner/ios/UIScriptControllerIOS.mm:
16+
(WTR::UIScriptControllerIOS::tapHighlightViewRect const):
17+
118
2021-04-09 Aakash Jain <aakash_jain@apple.com>
219

320
git-webkit find doesn't work well with unicode characters in author name (follow-up fix)

0 commit comments

Comments
 (0)