Bug 144451

Summary: [iOS] DOM click event may not be dispatched when page has :active style and <input type="search">
Product: WebKit Reporter: Michael "Spell" Spellacy <michael.spellacy>
Component: CSSAssignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, dbates, dino, eric.carlson, esprehn+autocc, glenn, jer.noble, kondapallykalyan, michael.spellacy, rniwa, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: 525.x (Safari 3.2)   
Hardware: iPhone / iPad   
OS: iOS 8.2   
URL: https://tmpworldwide.github.io/bugs/ios-tappy-bug.html
Bug Depends on:    
Bug Blocks: 152449    
Attachments:
Description Flags
[Web Archive] YouTube embed not working in iOS 8 Safari and Chrome
none
Patch and layout test
none
Archive of layout-test-results from ews101 for mac-mavericks
none
Archive of layout-test-results from ews107 for mac-mavericks-wk2
none
Patch and layout test
none
Archive of layout-test-results from ews106 for mac-mavericks-wk2
none
Archive of layout-test-results from ews100 for mac-mavericks
none
Patch and layout tests
none
[Alternative] Patch and layout tests
none
Patch and layout tests
none
Patch and layout tests none

Michael "Spell" Spellacy
Reported 2015-04-30 08:49:42 PDT
iPhone 5s Model: ME341LL/A iOS Version: 8.3 (12F0) Under certain conditions, YouTube videos embedded within iframes cannot be viewed when a user clicks on the video. In our particular instance, the conditions had to be just right. A "perfect storm" of issues, if you will. :) The Bug Applying the :active pseudo-class to a universal selector (*) and including a property of -webkit-tap-highlight-color seems to be the culprit. *:active { -webkit-tap-highlight-color: tomato; } Now, here is the strange part. The bug is only triggered when the above CSS block is present and there is an input element present on the page with a type attribute value of "search". I know, crazy, right? <input type="search" placeholder="Search"/> Another oddity here is that when you apply focus to the search input, type something in, and then attempt to play video, it will then work. The Solution Either renaming the input type to "text" or removing -webkit-tap-highlight-color (likely preferred solution) should alleviate the issue, but this obviously might be a rendering bug. You can the same overview here, and see the bug in action for yourself on any current iOS device. https://tmpworldwide.github.io/bugs/ios-tappy-bug.html
Attachments
[Web Archive] YouTube embed not working in iOS 8 Safari and Chrome (1.33 MB, application/x-webarchive)
2015-05-15 20:31 PDT, Daniel Bates 		no flags
Details
Patch and layout test (14.32 KB, patch)
2015-10-13 16:29 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews101 for mac-mavericks (1.57 MB, application/zip)
2015-10-13 17:02 PDT, Build Bot 		no flags
Details
Archive of layout-test-results from ews107 for mac-mavericks-wk2 (1.63 MB, application/zip)
2015-10-13 17:06 PDT, Build Bot 		no flags
Details
Patch and layout test (15.14 KB, patch)
2015-10-13 20:40 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews106 for mac-mavericks-wk2 (788.13 KB, application/zip)
2015-10-13 21:06 PDT, Build Bot 		no flags
Details
Archive of layout-test-results from ews100 for mac-mavericks (627.85 KB, application/zip)
2015-10-13 21:13 PDT, Build Bot 		no flags
Details
Patch and layout tests (35.13 KB, patch)
2015-10-14 18:24 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
[Alternative] Patch and layout tests (25.80 KB, patch)
2015-10-14 18:25 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch and layout tests (35.14 KB, patch)
2015-10-14 18:28 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch and layout tests (17.72 KB, patch)
2015-12-12 16:14 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Daniel Bates
Comment 1 2015-05-15 20:31:30 PDT
Created attachment 253260 [details] [Web Archive] YouTube embed not working in iOS 8 Safari and Chrome For historical preservation, a web archive of <https://tmpworldwide.github.io/bugs/ios-tappy-bug.html> as it appeared on 05/15/2015.
Daniel Bates
Comment 2 2015-10-13 16:25:35 PDT
This issue isn't specific to using the CSS property -webkit-tap-highlight-color. This issue can be reproduced on the example site (attachment #253260 [details]) with an arbitrary CSS property in the definition of *:active.
Daniel Bates
Comment 3 2015-10-13 16:25:40 PDT
On iOS we only dispatch a DOM click event if the content does not change as part of dispatching a DOM mousemove event at the tapped element. In particular, we do not dispatch a DOM click event if there is a visibility change to some element on the page as part of dispatching a mousemove event at the tapped element. Notice that we update the active state of elements as part of dispatching a mousemove event, forcing a full a style recalculation of affected elements, including setting its CSS property visibility to visible. And we update the visibility of the cancel button whenever its CSS property visibility disagrees with the CSS property visibility of its associated search field or the value of the search field changes. So, we always update the visibility of the cancel button since its CSS property visibility changes when updating the active state of the HTML body; => the content of the page changed => we do not dispatch a DOM click event.
Radar WebKit Bug Importer
Comment 4 2015-10-13 16:27:00 PDT
<rdar://problem/23099482>
Daniel Bates
Comment 5 2015-10-13 16:29:23 PDT
Created attachment 263037 [details] Patch and layout test
Daniel Bates
Comment 6 2015-10-13 16:48:56 PDT
Comment on attachment 263037 [details] Patch and layout test View in context: https://bugs.webkit.org/attachment.cgi?id=263037&action=review > Source/WebCore/ChangeLog:3 > + [iOS] DOM click event may not be dispatched when page has :active style and <input type="search">rch"> We remove trailing 'rch">' from title before landing. > LayoutTests/ChangeLog:3 > + [iOS] DOM click event may not be dispatched when page has :active style and <input type="search">rch"> Ditto.
Simon Fraser (smfr)
Comment 7 2015-10-13 16:57:42 PDT
Comment on attachment 263037 [details] Patch and layout test View in context: https://bugs.webkit.org/attachment.cgi?id=263037&action=review > Source/WebCore/ChangeLog:9 > + Without loss of generality, fixes an issue where a DOM click event is not dispatched to an "Without loss of generality" makes this sound like a maths thesis, and adds nothing. > Source/WebCore/ChangeLog:22 > + Test: platform/ios-simulator/ios/fast/events/can-click-element-on-page-with-active-pseudo-class-and-search-field.html We should not be adding new tests under platform. > Source/WebCore/ChangeLog:32 > + (WebCore::SearchInputType::updateCancelButtonVisibility): Added. Updates the CSS property display > + of the cancel button element such that it is visible when its associated search field is non-empty > + or the argument forceUpdate is true (defaults to false). We update the CSS property display instead > + of the CSS property visibility as a means to show and hide the cancel button because it is consistent > + with how we show and hide other buttons (e.g. the AutoFill button) and the former destroys the render > + tree associated with the cancel button when the cancel button is not shown, allowing its memory to be I would like this to justify why you moved the code from rendering to DOM.
Build Bot
Comment 8 2015-10-13 17:02:36 PDT
Comment on attachment 263037 [details] Patch and layout test Attachment 263037 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/280895 New failing tests: fast/forms/search-disabled-readonly.html fast/forms/searchfield-heights.html fast/forms/search/search-padding-cancel-results-buttons.html fast/forms/placeholder-pseudo-style.html fast/forms/search-vertical-alignment.html fast/forms/search-zoomed.html fast/replaced/width100percent-searchfield.html fast/forms/search-cancel-button-style-sharing.html fast/css/input-search-padding.html fast/forms/control-restrict-line-height.html fast/forms/search-abs-pos-cancel-button.html fast/forms/search-cancel-button-events.html fast/css/text-input-with-webkit-border-radius.html fast/forms/placeholder-position.html fast/forms/box-shadow-override.html accessibility/mac/search-field-cancel-button.html fast/forms/search-cancel-button-mouseup.html fast/forms/search-transformed.html fast/css/focus-ring-exists-for-search-field.html fast/forms/search-rtl.html fast/forms/search-styled.html fast/css/text-overflow-input.html fast/repaint/search-field-cancel.html fast/forms/input-appearance-height.html fast/forms/search/search-size-with-decorations.html
Build Bot
Comment 9 2015-10-13 17:02:41 PDT
Created attachment 263039 [details] Archive of layout-test-results from ews101 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5
Build Bot
Comment 10 2015-10-13 17:06:27 PDT
Comment on attachment 263037 [details] Patch and layout test Attachment 263037 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/280899 New failing tests: fast/forms/search-disabled-readonly.html fast/forms/searchfield-heights.html fast/forms/search/search-padding-cancel-results-buttons.html fast/forms/placeholder-pseudo-style.html fast/forms/search-vertical-alignment.html fast/forms/search-zoomed.html fast/replaced/width100percent-searchfield.html fast/forms/search-cancel-button-style-sharing.html fast/css/input-search-padding.html fast/forms/control-restrict-line-height.html fast/forms/search-abs-pos-cancel-button.html fast/forms/search-cancel-button-events.html fast/css/text-input-with-webkit-border-radius.html fast/forms/placeholder-position.html fast/forms/box-shadow-override.html accessibility/mac/search-field-cancel-button.html fast/forms/search-cancel-button-mouseup.html fast/forms/search-transformed.html fast/css/focus-ring-exists-for-search-field.html fast/forms/search-rtl.html fast/forms/search-styled.html fast/css/text-overflow-input.html fast/repaint/search-field-cancel.html fast/forms/input-appearance-height.html fast/forms/search/search-size-with-decorations.html
Build Bot
Comment 11 2015-10-13 17:06:31 PDT
Created attachment 263040 [details] Archive of layout-test-results from ews107 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Daniel Bates
Comment 12 2015-10-13 17:23:25 PDT
(In reply to comment #7) > Comment on attachment 263037 [details] > Patch and layout test > > View in context: > https://bugs.webkit.org/attachment.cgi?id=263037&action=review > > > Source/WebCore/ChangeLog:9 > > + Without loss of generality, fixes an issue where a DOM click event is not dispatched to an > > "Without loss of generality" makes this sound like a maths thesis, and adds > nothing. > Will remove. For completeness, I used this phrase to convey that defining a CSS :active pseudo-class explicitly on the HTML body is not necessary. > > Source/WebCore/ChangeLog:22 > > + Test: platform/ios-simulator/ios/fast/events/can-click-element-on-page-with-active-pseudo-class-and-search-field.html > > We should not be adding new tests under platform. > Will move tests to LayoutTests/fast/events. > > Source/WebCore/ChangeLog:32 > > + (WebCore::SearchInputType::updateCancelButtonVisibility): Added. Updates the CSS property display > > + of the cancel button element such that it is visible when its associated search field is non-empty > > + or the argument forceUpdate is true (defaults to false). We update the CSS property display instead > > + of the CSS property visibility as a means to show and hide the cancel button because it is consistent > > + with how we show and hide other buttons (e.g. the AutoFill button) and the former destroys the render > > + tree associated with the cancel button when the cancel button is not shown, allowing its memory to be > > I would like this to justify why you moved the code from rendering to DOM. Will justify movement of code.
Daniel Bates
Comment 13 2015-10-13 20:39:10 PDT
Comment on attachment 263037 [details] Patch and layout test View in context: https://bugs.webkit.org/attachment.cgi?id=263037&action=review > Source/WebCore/ChangeLog:31 > + (WebCore::SearchInputType::updateCancelButtonVisibility): Added. Updates the CSS property display > + of the cancel button element such that it is visible when its associated search field is non-empty > + or the argument forceUpdate is true (defaults to false). We update the CSS property display instead > + of the CSS property visibility as a means to show and hide the cancel button because it is consistent > + with how we show and hide other buttons (e.g. the AutoFill button) and the former destroys the render Actually, we need to update the CSS visibility property instead of using the CSS display property since we want the cancel button to take up space even when not visible so as to be consistent with the appearance of NSSearchField.
Daniel Bates
Comment 14 2015-10-13 20:40:53 PDT
Created attachment 263052 [details] Patch and layout test
Build Bot
Comment 15 2015-10-13 21:06:15 PDT
Comment on attachment 263052 [details] Patch and layout test Attachment 263052 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/281696 New failing tests: fast/forms/search-cancel-in-invisible-elements.html fast/events/can-click-element-on-page-with-active-pseudo-class-and-search-field.html
Build Bot
Comment 16 2015-10-13 21:06:20 PDT
Created attachment 263054 [details] Archive of layout-test-results from ews106 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Build Bot
Comment 17 2015-10-13 21:13:09 PDT
Comment on attachment 263052 [details] Patch and layout test Attachment 263052 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/281726 New failing tests: fast/forms/search-cancel-in-invisible-elements.html
Build Bot
Comment 18 2015-10-13 21:13:13 PDT
Created attachment 263055 [details] Archive of layout-test-results from ews100 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-mavericks Platform: Mac OS X 10.9.5
Daniel Bates
Comment 19 2015-10-14 18:24:12 PDT
Created attachment 263128 [details] Patch and layout tests Updated patch to update the visibility of the cancel button when the visibility of the search field is changed programmatically. Added tests for such changes as well as to ensure we update the cancel button (if needed) when the search field is disabled or made read-only.
Daniel Bates
Comment 20 2015-10-14 18:25:50 PDT
Created attachment 263129 [details] [Alternative] Patch and layout tests An alternative patch to fix this issue that keeps the cancel button visibility logic in RenderSearchField.
Daniel Bates
Comment 21 2015-10-14 18:28:00 PDT
Created attachment 263130 [details] Patch and layout tests Rebased patch.
Daniel Bates
Comment 22 2015-10-15 09:09:02 PDT
Committed r191113: <http://trac.webkit.org/changeset/191113>
Daniel Bates
Comment 23 2015-10-22 15:11:05 PDT
(In reply to comment #22) > Committed r191113: <http://trac.webkit.org/changeset/191113> This regressed the visibility of the search cancel button when the search field is empty or showing placeholder text.
Daniel Bates
Comment 24 2015-10-22 15:15:13 PDT
Reverted r191113 for reason: Rollout r144451 since it regressed the visibility of the search cancel button when a search field is empty or showing placeholder text. Further investigation is needed. Committed r191480: <http://trac.webkit.org/changeset/191480>
Csaba Osztrogonác
Comment 25 2015-11-17 02:50:46 PST
Comment on attachment 263052 [details] Patch and layout test Cleared Simon Fraser's review+ from obsolete attachment 263052 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Daniel Bates
Comment 26 2015-12-12 16:14:36 PST
Created attachment 267247 [details] Patch and layout tests Updated patch to ignore visibility changes to user agent shadow DOM elements, such as the search field cancel button.
WebKit Commit Bot
Comment 27 2015-12-14 10:07:45 PST
Comment on attachment 267247 [details] Patch and layout tests Clearing flags on attachment: 267247 Committed r194038: <http://trac.webkit.org/changeset/194038>
WebKit Commit Bot
Comment 28 2015-12-14 10:07:51 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.