[webkit-reviews] review denied: [Bug 90498] add a setting to allow user to play a media by clicking on it : [Attachment 150691] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has denied Min Qin
<qinmin at chromium.org>'s request for review:
Bug 90498: add a setting to allow user to play a media by clicking on it
https://bugs.webkit.org/show_bug.cgi?id=90498

Attachment 150691: Patch
https://bugs.webkit.org/attachment.cgi?id=150691&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=150691&action=review


> Source/WebCore/html/HTMLMediaElement.cpp:3855
> +    if (document()->settings() &&
document()->settings()->mediaClickToPlayEnabled() && event->type() ==
eventNames().clickEvent && canPlay()) {
> +	   play();
> +	   event->setDefaultHandled();

This strikes me as the wrong way to enable this feature. Other (most?) players
that enable "click to play" have a UI affordance to help the user understand
that clicking will initiate playback, often a large "play" button. If someone
wanted to implement that in a WebKit port, they would not be able to use this
setting because the play button would intercept the click, and they would
presumably want to restrict the behavior to clicks in the play button.

It seems to me that you are adding a new control function, so the right place
for this is in the media control code. It should be easy to add an element to
the controls shadow DOM and do the hit testing there. Whether you actually show
a button or not is tangential.