1484544 - Input element unusable on top level video document

Status: VERIFIED FIXED

(Toolkit :: Video/Audio Controls, defect)

Description

[hanu6]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID: 20100101

Steps to reproduce:

In its own tab, open a video (I used a webm in Firefox developer edition 62.0b17 64bit).

Append an input element with javascript.
let inp = document.createElement('input')
inp.type='text'
document.body.append(inp)


Actual results:

The input element is unusable and does not take text.


Expected results:

Input should work as normal.

Comment 1

[u554753]

Hi hanu6,

Thanks for reporting the issue. Can you attach a full test case of the issue?

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Works fine here on Nightly FWIW. What's the use-case for this?

Comment 3

[Daniel Holbert [:dholbert] (PTO until Jun 29)]

(In reply to Grover Wimberly IV [:Grover-QA] from comment #1)
> Thanks for reporting the issue. Can you attach a full test case of the issue?

Sounds like a testcase isn't possible here. The steps in comment 0 are sufficient to test, if you've got a video to load.  Here's a sample video that works: http://media.tinyvid.tv/lhbt7kl3n3pf.ogg


(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)
> Works fine here on Nightly FWIW. What's the use-case for this?

I can reproduce the issue in Nightly on Linux.  I'm curious about the use case, too, though... This is an internal bit of Firefox UI, which isn't expecting to have its DOM messed with.

Looking in DevTools, I see that our top-level video document has an event handler on the root element, which redirects all focus to the video controls. The code for that is here (and was added in bug 1483408 it seems):
https://searchfox.org/mozilla-central/rev/3fa761ade83ed0b8ab463acb057c2cf0b104689e/toolkit/content/TopLevelVideoDocument.js#13-26

That chunk clearly is going to mean that other elements in the web page won't be able to receive focus -- which shouldn't normally be a problem, because this is a bit of Firefox internal UI that shouldn't have any extra elements, beyond those that the user adds themselves via manual DOM surgery like that described in comment 0. :)

Comment 4

[Daniel Holbert [:dholbert] (PTO until Jun 29)]

Some clearer STR, just so we're all using the same steps:

 1. Visit http://media.tinyvid.tv/lhbt7kl3n3pf.ogg

 2. Ctrl+Shift+K / Cmd+Shift+K to open console (or "Tools|Web Developer|Web Console)

 3. Paste the following line into the console and hit enter:
   document.body.append(document.createElement('input'))

 4. Try to click to focus the textfield that you'll now see on the page.

Comment 5

[hanu6]

Use case is add-ons.

I have an addon that creates an input element so you can mess around with settings specific to the video.

For reference: https://github.com/losnappas/Custom-Loop/issues/4

STR is exactly like comment #4.

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Interesting case!

So the Web spec does allow the implementation to put any script in the media document (see bug 1474832), but I do agree this can be improved.

Perhaps we could install a mutation observer there and remove the focus event listener as soon as it detects any DOM changes.

This bug needs to be fixed soon if we don't want this to reach release.

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I'll just push a patch for this then.

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: Bug 1484544 - Allow focusable element to function on top level video element r=Gijs

Comment 9

[:Gijs (he/him)]

Comment on attachment 9003640 [details]
Bug 1484544 - Allow focusable element to function on top level video element r=Gijs

:Gijs (he/him) has approved the revision.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a28cf3253e23
Allow focusable element to function on top level video element r=Gijs

Comment 11

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/a28cf3253e23

Comment 12

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 9003640 [details]
Bug 1484544 - Allow focusable element to function on top level video element r=Gijs

Approval Request Comment
[Feature/Bug causing the regression]: bug 1483408
[User impact if declined]: Add-ons would not be able to insert DOM element on to top level media document that needs focus
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: See comment 4.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Patch only affect top level media document.
[String changes made/needed]:

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9003640 [details]
Bug 1484544 - Allow focusable element to function on top level video element r=Gijs

62 is on release now, switching the request accordingly.

Comment 14

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

I managed to reproduce the initial issue on 62.0b20 build1 (20180823143155), using the STR provided in comment 4. I can confirm that 63.0a1 (2018-08-26) is verified fixed across platforms (Windows 10 x64, Ubuntu 16.04 x64, macOS 10.13.6). There is still a problem - Devedition 63.0b1 build1 (20180824192747) is still affected.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

That's expected given the timing of when this hit m-c. 63.0b2 should be OK.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9003640 [details]
Bug 1484544 - Allow focusable element to function on top level video element r=Gijs

Fixes a regression from bug 1483408. Approved for 62 RC1.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1dd281f0b1bd (FIREFOX_62b_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/c37fe5e636b7

Comment 18

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

The fix was successfully implemented on Devedition 63.0b2 build1 (20180830123124) and on 62.0 build2 (20180830143136) across platforms (macOS 10.13.6, Ubuntu 16.04 x64, Windows 10 x64). Setting the flags accordingly.