Bug 144451 – [iOS] DOM click event may not be dispatched when page has :active style and <input type="search">

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Michael "Spell" Spellacy]

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

[Daniel Bates]

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]

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]

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]

<rdar://problem/23099482>

[Daniel Bates]

Created attachment 263037 [details]
Patch and layout test

[Daniel Bates]

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 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 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]

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 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]

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]

(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 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]

Created attachment 263052 [details]
Patch and layout test

[Build Bot]

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]

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 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]

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]

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]

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]

Created attachment 263130 [details]
Patch and layout tests

Rebased patch.

[Daniel Bates]

Committed r191113: <http://trac.webkit.org/changeset/191113>

[Daniel Bates]

(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]

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 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]

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 on attachment 267247 [details]
Patch and layout tests

Clearing flags on attachment: 267247

Committed r194038: <http://trac.webkit.org/changeset/194038>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.