Don't allow text selection to start on a HTMLMediaElement

Don't allow text selection to start on a HTMLMediaElement

BUG=449146
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196741

[Mathias Hällman, 2015-01-15 14:42:00]

mathiash@opera.com changed reviewers:
+ philipj@opera.com

[philipj_slow, 2015-01-16 12:28:49]

The sensible behavior is for it to never be possible to start selecting by
clicking anywhere in an audio or video element, and it looks like overriding
HTMLMediaElement::canStartSelection() will work because
Node::canStartSelection() traverses all parents.

I see that there's some #if OS(ANDROID) in EventHandler::handleGestureLongPress,
so testing the original problem seems too hard. However, on desktop with a case
like "words before <video controls></video> words after" it's possible to start
selecting by clicking in the middle of the video, but not if you remove the
controls attribute. You should be able to write a test case for that, something
like the fast/dom/shadow/select-image-with-shadow.html test case that once
existed:
http://trac.webkit.org/changeset/125397

[philipj_slow, 2015-01-21 13:52:44]

Looks like fast/forms/label/label-contains-other-interactive-content.html needs
updating:
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...

The test clicks on a video with controls, but doesn't move the between mousedown
and mouseup. Not sure what's going on.

[philipj_slow, 2015-01-21 13:53:26]

The change and test LGTM assuming the test failure is not indicative of some
bigger problem.

[Mathias Hällman, 2015-04-23 14:46:16]

@tkent, could you give some input on the failing test? Is it ok to edit the
expectations? Otherwise my best bet is some entirely unrelated bug in
EventHandler.cpp, possibly something with m_mouseDownMayStartSelect.

[philipj_slow, 2015-04-23 14:57:55]

philipj@opera.com changed reviewers:
+ tkent@chromium.org

[philipj_slow, 2015-04-23 14:57:55]

Adding tkent@ to reviewers, not sure it will be noticed otherwise.

[tkent, 2015-04-23 23:09:20]

On 2015/04/23 14:46:16, Mathias Hällman wrote:
> @tkent, could you give some input on the failing test?

> Is it ok to edit the
> expectations?

No.  It's not acceptable.

> Otherwise my best bet is some entirely unrelated bug in
> EventHandler.cpp, possibly something with m_mouseDownMayStartSelect.

I don't have a clear idea for now.
HTMLLabelElement::defaultEventHandler has some logic for selection.  Please
investigate what happens in this function with this CL.

[philipj_slow, 2015-06-08 15:41:52]

The CQ bit was checked by philipj@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-06-08 15:41:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/838003005/20001

[commit-bot: I haz the power, 2015-06-08 16:33:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-08 16:33:14]

Dry run: This issue passed the CQ dry run.

[philipj_slow, 2015-06-08 18:20:53]

After minimizing the failing test case it turned out to be an unrelated bug
causing the audio element to not get its layout invalidated as part of
fast/forms/label/label-contains-other-interactive-content.html, reported and
fixed in https://codereview.chromium.org/1166743004/

With and without the changes in this CL, that resulted in a double click which
created a selection. The behavior when there's a selection is different when
canStartSelection() returns false, but it is the same behavior as for image and
does not seem anomalous to me.

tkent@, the bots looks happy with the other fix landed, but can you check if you
are satisfied with the root cause analysis and fix?

[tkent, 2015-06-08 23:26:07]

lgtm

[Mathias Hällman, 2015-06-09 07:16:26]

The CQ bit was checked by mathiash@opera.com

[commit-bot: I haz the power, 2015-06-09 07:16:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/838003005/20001

[commit-bot: I haz the power, 2015-06-09 07:20:16]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196741