Last Comment Bug 449358 - Add test to confirm that video/audio UI controls function when JavaScript is disabled
: Add test to confirm that video/audio UI controls function when JavaScript is ...
Status: RESOLVED FIXED
[gs][fixed by bug 834697]
: testcase
Product: Toolkit
Classification: Components
Component: Video/Audio Controls (show other bugs)
: unspecified
: All All
: -- normal with 24 votes (vote)
: mozilla21
Assigned To: Jared Wein [:jaws] (please needinfo? me)
:
: Jared Wein [:jaws] (please needinfo? me)
Mentors:
http://getsatisfaction.com/mozilla_me...
: 480552 499722 501325 526748 550557 604599 608971 639582 726740 (view as bug list)
Depends on: 236839 1272646 550557 834697
Blocks: 515082 TrackAVUI
  Show dependency treegraph
 
Reported: 2008-08-05 23:34 PDT by Biju
Modified: 2016-05-13 05:48 PDT (History)
60 users (show)
roc: blocking1.9.2-
dholbert: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
testcase (135 bytes, text/html)
2008-11-11 16:16 PST, Daniel Holbert [:dholbert]
no flags Details
Testcase (3.17 KB, patch)
2013-02-15 13:39 PST, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Splinter Review
Test (3.10 KB, patch)
2016-05-12 09:57 PDT, Jared Wein [:jaws] (please needinfo? me)
gijskruitbosch+bugs: review+
Details | Diff | Splinter Review
Patch for check-in (3.21 KB, patch)
2016-05-12 20:41 PDT, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Splinter Review

Description Biju 2008-08-05 23:34:03 PDT
http://www.whatwg.org/specs/web-apps/current-work/#controls

>>>> 
The controls attribute is a boolean attribute. If the attribute is present, or if the media element is without script, then the user agent should expose a user interface to the user. 
<<<<

I dont see controls when JavaScript is disabled for
  <video src=support/video.ogg></video>

Steps:
1. From menu goto Tools > Options > Content
2. uncheck "Enable Javascript"
3. Goto URL http://simon.html5.org/test/html/semantics/video/006.htm

Result:-
User dont see controls

Expected:-
There should be controls on video element.
Comment 1 Met - Martin Hassman 2008-09-20 06:22:22 PDT
Note: Same applies to audio element too.
Comment 2 Biju 2008-11-02 15:59:18 PST
Behaviour of BUG CHANGED !!!
From: No controls when JavaScript is disabled  
===> Video/Audio UI Controls not functioning when JavaScript is disabled


This bug is partially fixed now...
(may be after patch related to bug 448909 or bug 449522)
Currently user see non-functioning controls over VIDEO.

Steps:
1. From menu goto Tools > Options > Content
2. uncheck "Enable Javascript"
3. Goto URL http://simon.html5.org/test/html/semantics/video/006.htm
4. User see controls

Result:-
But it is not functioning.
a) Pause button is shown instead of play
b) clicking any controls do nothing
c) play video thru context menu, still controls not functioning
d) controls dont disappear after mouse leave video area

Expected:-
Controls should behave the same way as JS enabled.
Comment 3 Justin Dolske [:Dolske] 2008-11-11 16:14:27 PST
The script in the XBL hides the controls on init, so if the script isn't being run they'll be visible by default. (Conversely, if the  controls were hidden by default, they'd never become visible if its script couldn't run.)
Comment 4 Daniel Holbert [:dholbert] 2008-11-11 16:16:31 PST
Created attachment 347660 [details]
testcase

The links given in comment 0 and comment 2 are broken.  To prevent further links becoming broken, I'm attaching a bugzilla-hosted HTML testcase that uses the <video> element.

The video src is a random ogg attachment I found on bugzilla:
  https://bugzilla.mozilla.org/attachment.cgi?id=318816
which is a short screencast of a user interacting with a firefox window.
Comment 5 Daniel Holbert [:dholbert] 2008-11-11 16:22:48 PST
Here's one other interesting point (with scripting disabled):
 - If I load the HTML testcase (attachment 347660 [details]) ...
   -- Result: I see frozen screenshot of the video, with controls overlayed.

 - If I load OGG directly (attachment 318816 [details]) ...
   -- Result: I see a **blank page**, with controls overlayed.

It seems odd that these two experiments don't have the same results.
Comment 6 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-11-13 20:16:04 PST
I'm not sure what we can do here :-(.

We can probably override the script-enabled setting for stand-alone video documents, I guess. But I'm not sure how to do that.

Fixing this for regular <video> elements sounds really hard.
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2008-11-14 10:41:02 PST
> I'm not sure what we can do here :-(.

Well... take this into account when making the decision to use the brokenness that is XBL for anything in content?  :(

I suppose we could try just fixing bug 236839...  It might be doable if we add some extra args to nsIScriptContext to override the script-enabled checks it does and then carefully call them from XBL as needed.  We'd still have issues with XBL-attached event handlers, though, without a bit more work.  :(
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2008-11-14 10:42:13 PST
For overriding for standalone documents, you could add some hackery to security manager, I guess...  The thing is, is that safe?  Someone could inject nodes into that document via DOM manipulation, say.
Comment 9 Daniel Holbert [:dholbert] 2008-11-14 10:49:59 PST
(In reply to comment #8)
> For overriding for standalone documents, you could add some hackery to security
> manager, I guess...

Would that fix this when scripts are blocked via NoScript, too?  (That's how I originally ran into this bug, and I expect it's how most users would end up running into it.)
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2008-11-14 11:38:25 PST
I think it should, yes.
Comment 11 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-11-14 12:00:32 PST
(In reply to comment #8)
> For overriding for standalone documents, you could add some hackery to security
> manager, I guess...  The thing is, is that safe?  Someone could inject nodes
> into that document via DOM manipulation, say.

They can only do that with script, but it does sound like they might be able to survive JS being disabled that way.
Comment 12 Boris Zbarsky [:bz] (still a bit busy) 2008-11-14 12:03:38 PST
Also, one site can use script + privileges to inject script that would then run as a different site (for which you have script disabled).  Not a common attack scenario, but still...
Comment 13 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-11-14 12:04:35 PST
(In reply to comment #7)
> Well... take this into account when making the decision to use the brokenness
> that is XBL for anything in content?  :(

We could reimplement it all in C++ I guess. But that sucks pretty bad. I'm not sure if Justin's going to want to keep working on it there.

> I suppose we could try just fixing bug 236839...  It might be doable if we add
> some extra args to nsIScriptContext to override the script-enabled checks it
> does and then carefully call them from XBL as needed.  We'd still have issues
> with XBL-attached event handlers, though, without a bit more work.  :(

That sounds like a lot of work, especially to make secure, but probably something we want XBL2 to be able to handle.

I wonder if we can get away with leaving this unfixed in this release. Maybe we can hack things a little bit so that if script is disabled, controls don't show up.
Comment 14 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-11-14 12:06:53 PST
(In reply to comment #12)
> Also, one site can use script + privileges to inject script that would then run
> as a different site (for which you have script disabled).  Not a common attack
> scenario, but still...

If the site has privileges, can't it just enable script wherever and however it wants? Like anti-Noscript
Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2008-11-14 12:09:08 PST
Yeah, I don't know that we have any good options here.  That's why we need xbl2...

Hacking things up to not show the controls should be easy, I'd think.

Privileges needed for content injection are lower than UniversalXPConnect at the moment.  But yes, the whole thing is maybe just me being paranoid.
Comment 16 Justin Dolske [:Dolske] 2008-11-14 13:31:14 PST
(In reply to comment #13)

> We could reimplement it all in C++ I guess. But that sucks pretty bad. I'm not
> sure if Justin's going to want to keep working on it there.

My preference would indeed be to keep it as JS/XUL, since that's what the rest of the front end is, and it makes it more approachable to other front-end folks.
Perhaps some kind of hybrid approach might be possible... EG, a bit of C++ to hook up the events to a JS component instead of code in the binding.

> That sounds like a lot of work, especially to make secure, but probably
> something we want XBL2 to be able to handle.

Would it at all help to do something as a special-case hack, unique to "chrome://global/content/bindings/videocontrols.xml"? Evil, but maybe a limited-scope bandaid could help mitigate risk.

> I wonder if we can get away with leaving this unfixed in this release. Maybe we
> can hack things a little bit so that if script is disabled, controls don't show
> up.

Seems like we probably could get away with leaving this broken for 1 release... Especially if fixing now would require a large and/or risky change. The context menu is there, which is enough to play/pause/mute, so it's not totally broken.

Having the controls default to being hidden should be easy to change.
Comment 17 Boris Zbarsky [:bz] (still a bit busy) 2008-11-14 17:11:35 PST
> Would it at all help to do something as a special-case hack

If we could do that it'd be just as easy to do it for all chrome:// (so fix bug 236839).
Comment 18 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-11-14 23:30:22 PST
(In reply to comment #16)
> Perhaps some kind of hybrid approach might be possible... EG, a bit of C++ to
> hook up the events to a JS component instead of code in the binding.

Maybe but then you'd be dealing with chrome wrappers and stuff. I'm not sure how feasible that is.

> Having the controls default to being hidden should be easy to change.

Let's do that at least.
Comment 19 Justin Dolske [:Dolske] 2008-12-23 18:46:35 PST
(In reply to comment #18)

> > Having the controls default to being hidden should be easy to change.
> 
> Let's do that at least.

My patch in bug 469211 will do that.
Comment 20 Jason Oster (:Parasyte) 2009-02-27 07:59:59 PST
*** Bug 480552 has been marked as a duplicate of this bug. ***
Comment 21 Jason Oster (:Parasyte) 2009-02-27 08:14:13 PST
*** Bug 480552 has been marked as a duplicate of this bug. ***
Comment 22 Steven Rowat 2009-06-22 17:32:14 PDT
*** Bug 499722 has been marked as a duplicate of this bug. ***
Comment 23 Matthias Versen [:Matti] 2009-06-30 08:43:59 PDT
*** Bug 501325 has been marked as a duplicate of this bug. ***
Comment 24 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-07-30 22:01:40 PDT
Can't block on this, there is no reasonable way to fix it. Save us XBL2!
Comment 25 scientus 2009-09-17 06:06:38 PDT
This bug seems to be dependent on bug 236839
Comment 26 Joshua Cranmer [:jcranmer] 2009-11-05 05:12:33 PST
*** Bug 526748 has been marked as a duplicate of this bug. ***
Comment 27 cajbir (:cajbir) 2010-10-15 03:13:19 PDT
*** Bug 604599 has been marked as a duplicate of this bug. ***
Comment 28 Kevin Brosnan 2010-11-02 07:17:34 PDT
*** Bug 608971 has been marked as a duplicate of this bug. ***
Comment 29 Marat Tanalin | tanalin.com 2011-02-28 10:23:00 PST
Currently, audio elements is not displaying _at all_ when JavaScript is disabled. This is like as if, say, hyperlinks would not work when JavaScript is disabled. Just nonsensical.

So, current support for audio/video elements in Firefox is more like a temporary _workaround_ instead of a real native implementation. Very sad.

In Opera and Chrome, audio element DOES work regardless of whether JS is enabled or disabled, and that is really correct implementation.

Hopefully to see this fixed in Firefox as soon as possible. Thanks.
Comment 30 Jan D 2011-03-15 09:36:49 PDT
*** Bug 639582 has been marked as a duplicate of this bug. ***
Comment 31 Robert Longson 2012-02-13 13:42:50 PST
*** Bug 726740 has been marked as a duplicate of this bug. ***
Comment 32 John Drinkwater (:beta) 2012-06-11 06:07:08 PDT
PDF.js is similarly disabled by NoScript even though it, like video/audio, is code hosted by the browser…
Comment 33 onpon4 2013-02-05 10:51:16 PST
With videos, there are absolutely no controls if Javascript is disabled as of version 18.0.1. I don't think this was the case with version 16, the last version I used before 18.0.1.

This is silly. It really ought to be fixed.
Comment 34 Daniel Holbert [:dholbert] 2013-02-05 11:03:30 PST
(In reply to onpon4 from comment #33)
> With videos, there are absolutely no controls if Javascript is disabled as
> of version 18.0.1. I don't think this was the case with version 16, the last
> version I used before 18.0.1.

I don't think our behavior has changed on this recently.  I see the same behavior (no controls) in the attached testcase, in both trunk and in Firefox 9.0 (which I happen to have handy), when scripts are disabled.

(It does look like this is different from what was reported in Comment 0, thoguh -- there, we had visible-but-non-functional controls.  So, I think the behavior has changed, but it was a long time ago.)

Incidentally, I've noticed that the context menu controls do work, however (Right-click video --> Play/Pause), in trunk as well as in an old build (ver 9.0)
Comment 35 John Drinkwater (:beta) 2013-02-05 12:12:46 PST
(In reply to Daniel Holbert [:dholbert] from comment #34)
> I don't think our behavior has changed on this recently.  I see the same
> behavior (no controls) in the attached testcase, in both trunk and in
> Firefox 9.0 (which I happen to have handy), when scripts are disabled.

Something in PDFjs changed recently to get around this limitation, as now I can view pdfs without enabling the domain in noscript. Wonder if that can be ported over.
Comment 36 Jared Wein [:jaws] (please needinfo? me) 2013-02-05 12:26:07 PST
Brendan, do you know anything about comment #35?
Comment 37 Robert O'Callahan (:roc) (email my personal email if necessary) 2013-02-06 15:44:57 PST
(In reply to John Drinkwater (:beta) from comment #35)
> Something in PDFjs changed recently to get around this limitation, as now I
> can view pdfs without enabling the domain in noscript. Wonder if that can be
> ported over.

It cannot. PDF.js doesn't have to run in the context of a Web page. Video controls do.

This bug has existed since we first implemented video controls. The good news is that the work in bug 821850 will make it possible to fix this bug. We need to wait for that to land first.
Comment 38 Jared Wein [:jaws] (please needinfo? me) 2013-02-15 08:20:37 PST
I just tested this out with Tools->Options->Content->Enable Javascript unchecked, restarted the browser, and visited http://mirrorblender.top-ix.org/peach/bigbuckbunny_movies/big_buck_bunny_1080p_stereo.ogg.

The controls now work.

Bug 834697 fixed this.
Comment 39 John Drinkwater (:beta) 2013-02-15 08:49:40 PST

    
Comment 40 Boris Zbarsky [:bz] (still a bit busy) 2013-02-15 10:29:47 PST
Can we please add a test for this?  E.g. there's a good chance bug 835643 or the like would re-break this....
Comment 41 Jared Wein [:jaws] (please needinfo? me) 2013-02-15 13:39:39 PST
Created attachment 714565 [details] [diff] [review]
Testcase

This is a basic testcase that disables JS, opens a new window, and uses the overlay play button to pause the video. I confirmed locally that if I set dom.xbl_scopes=false then the test fails (it hangs).

https://tbpl.mozilla.org/?tree=Try&rev=16805b025240
Comment 42 Loic 2013-02-15 13:42:15 PST
Should video controls appear if an add-on like NoScript blocks JS?
Comment 43 Jeff Walden [:Waldo] (remove +bmo to email) 2013-02-15 13:57:10 PST
Yes -- nothing should ever prevent video controls from displaying and being functional/responsive to user manipulation.
Comment 44 Jared Wein [:jaws] (please needinfo? me) 2013-02-15 15:49:09 PST
Comment on attachment 714565 [details] [diff] [review]
Testcase

The test is failing on try, so more work needs to be done here.
Comment 45 Daniel Holbert [:dholbert] 2013-02-16 11:49:19 PST
[removing bug 834697 from "see also". As that field's hover-text indicates, it isn't just for linking to related bugs -- rather, it's specifically for *reports of this same bug in other bug-trackers* -- e.g. if a Firefox bug turns out to be a Gnome bug, or something like that, then that would be an appropriate place to link to the Gnome bug page.  Related mozilla bugs should be tracked as dependencies or duplicates, not via "see also". In this case, bug 834697 is already in this bug's "depends on" field, so we're good.]
Comment 46 John Drinkwater (:beta) 2013-02-17 02:26:04 PST
(In reply to Loic from comment #42)
> Should video controls appear if an add-on like NoScript blocks JS?
That appears to be the case in the latest nightly, with NoScript enabled and the domain untrusted, controls still appear and are usable.

The URL supplied in comment #1 hasn’t aged with spec/useragents - it is missing controls be default - so probably no longer a useful testcase to check.
Comment 47 Jeff Walden [:Waldo] (remove +bmo to email) 2013-02-17 18:41:18 PST
(In reply to Daniel Holbert [:dholbert] from comment #45)
> [removing bug 834697 from "see also". As that field's hover-text indicates,
> it isn't just for linking to related bugs -- rather, it's specifically for
> *reports of this same bug in other bug-trackers*

The tooltip might say that, but I'm pretty sure that's not how I've seen it exclusively used.  However, I'm too lazy to search them out right now to conclusively rebut the point.  :-)
Comment 48 Bobby Holley (:bholley) (busy with Stylo) 2013-03-11 10:55:46 PDT
Reopening and assigning to jaws per comment 44.
Comment 49 neil@parkwaycc.co.uk 2013-05-28 07:13:41 PDT
(In reply to Jared Wein from comment #38)
> Bug 834697 fixed this.
(Unless you whitelisted XBL on the site.)
Comment 50 Jared Wein [:jaws] (please needinfo? me) 2014-05-15 13:45:23 PDT
Unassigning as I haven't been working on this and I don't want to hold people up.
Comment 51 Jared Wein [:jaws] (please needinfo? me) 2016-05-05 08:28:56 PDT
*** Bug 550557 has been marked as a duplicate of this bug. ***
Comment 52 antistress 2016-05-10 15:17:58 PDT
Since Electrolysis is about using different processes for UI and web content, maybe that will solve that bug (=cutting JavaScript for web content may not affect the UI ?)
Comment 53 Boris Zbarsky [:bz] (still a bit busy) 2016-05-10 20:14:51 PDT
The UI that runs in the parent process is the browser chrome (url bar, back/forward button, etc).  The video controls would still be in the content process.

That said, as far as I can tell this bug as filed is no longer a problem because the controls run in the separate XBL scope where script is not disabled.  The only reason this bug is still open is to get an automated test checked in.
Comment 54 Jared Wein [:jaws] (please needinfo? me) 2016-05-12 09:57:46 PDT
Created attachment 8751816 [details] [diff] [review]
Test

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe19145fe813
Comment 55 Jared Wein [:jaws] (please needinfo? me) 2016-05-12 12:30:43 PDT
Comment on attachment 8751816 [details] [diff] [review]
Test

This test is written in the same style as the pre-existing test_videocontrols_*.html files, and I basically rebased the previous attachment and changed from using the "play" event to the "timeupdate" event. I would prefer not to rewrite it using the latest conventions, but I am open ears.
Comment 56 :Gijs Kruitbosch 2016-05-12 14:58:43 PDT
Comment on attachment 8751816 [details] [diff] [review]
Test

Review of attachment 8751816 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/content/tests/widgets/test_videocontrols_jsdisabled.html
@@ +11,5 @@
> +<pre id="test">
> +<script class="testbody" type="text/javascript">
> +
> +function runTest(event) {
> +  ok(true, "----- test #" + testnum + " -----");

Nit: info()

@@ +43,5 @@
> +
> +  testnum++;
> +}
> +
> +SpecialPowers.setBoolPref("javascript.enabled", false);

I'm fairly suspicious that this does not in fact turn off JS. Either way I think we should be using pushPrefEnv to avoid this leaking to other tests if the test fails.

Can you add a <script> thing in the data URI that sets an expando on the window, and then check that the expando is *not* present when the load has happened ?

@@ +56,5 @@
> +  video.addEventListener("pause", runTest, false);
> +}
> +
> +var a = document.createElement("a");
> +a.href = "seek_with_sound.ogg";

Nit:

var videoURL = new URL("seek_with_sound.ogg", document.documentURI).href;

and then include that in the link.
Comment 57 :Gijs Kruitbosch 2016-05-12 15:01:59 PDT
(In reply to :Gijs Kruitbosch from comment #56)
> @@ +43,5 @@
> > +
> > +  testnum++;
> > +}
> > +
> > +SpecialPowers.setBoolPref("javascript.enabled", false);
> 
> I'm fairly suspicious that this does not in fact turn off JS. Either way I
> think we should be using pushPrefEnv to avoid this leaking to other tests if
> the test fails.
> 
> Can you add a <script> thing in the data URI that sets an expando on the
> window, and then check that the expando is *not* present when the load has
> happened ?

To clarify slightly: I think it might work right now (I'm not sure) but when behaviour was changed so that it no longer turned JS off that was already running (ie pages that were open before you changed the pref), people complained. From what I recall, it isn't particularly "supported" as a thing to do, and I would hate for the test to basically not be testing anything.
Comment 58 Jared Wein [:jaws] (please needinfo? me) 2016-05-12 20:41:21 PDT
Created attachment 8752033 [details] [diff] [review]
Patch for check-in
Comment 59 Jared Wein [:jaws] (please needinfo? me) 2016-05-12 20:43:37 PDT
(In reply to :Gijs Kruitbosch from comment #57)
> (In reply to :Gijs Kruitbosch from comment #56)
> > @@ +43,5 @@
> > > +
> > > +  testnum++;
> > > +}
> > > +
> > > +SpecialPowers.setBoolPref("javascript.enabled", false);
> > 
> > I'm fairly suspicious that this does not in fact turn off JS. Either way I
> > think we should be using pushPrefEnv to avoid this leaking to other tests if
> > the test fails.
> > 
> > Can you add a <script> thing in the data URI that sets an expando on the
> > window, and then check that the expando is *not* present when the load has
> > happened ?
> 
> To clarify slightly: I think it might work right now (I'm not sure) but when
> behaviour was changed so that it no longer turned JS off that was already
> running (ie pages that were open before you changed the pref), people
> complained. From what I recall, it isn't particularly "supported" as a thing
> to do, and I would hate for the test to basically not be testing anything.

Right, I agree. A misleading test is worse than no test at all. I needed to rely on the pushPrefEnv callback to wait long enough for JS to be disabled in the new tab. I confirmed that the test passed and failed with/without the pushPrefEnv call, respectively.
Comment 61 Carsten Book [:Tomcat] 2016-05-13 03:03:31 PDT
https://hg.mozilla.org/mozilla-central/rev/51d7f9e94442

Note You need to log in before you can comment on or make changes to this bug.