Closed Bug 1350947 Opened 8 years ago Closed 7 years ago

Postpone loading NPAPI Flash content until the tab is resumed

Categories

(Core Graveyard :: Plug-ins, enhancement)

Other Branch
enhancement
Not set
normal

Tracking

(firefox55 wontfix, firefox56 ?)

RESOLVED WONTFIX
Tracking Status
firefox55 --- wontfix
firefox56 --- ?

People

(Reporter: alwu, Assigned: alwu)

References

Details

Attachments

(1 file)

Now we can mute the NPAPI, but can't pause it. Could we support pause for NPAPI? The use cases are like bug1348759, bug1349202 and bug1348879, we would like pause NPAPI until the tab goes to foreground.
Because I don't very understand how NPAPI works, could you give me some suggestion or let me know whether it's workable or not? Thanks!
Flags: needinfo?(ehsan)
Flags: needinfo?(amarchesini)
There is an existing NPAPI signal that tells Flash that it is invisible and should stop painting (we send it a 0x0 window). Doing anything beyond that would require the cooperation of Flash. Given the low staffing level of Flash, it is unlikely that we will get significant improvements here. Most of our efforts around Flash are focused on deprecation and click-to-activate by default, which will push most major sites away from Flash anyway.
Summary: Support "pause" for NPAPI → Support "pause" for NPAPI Flash
I think Benjamin gave a perfect description of what we should do for this bug. Basically, you should contact somebody from the Flash team in adobe and ask if they want to introduce this feature. If yes, we can continue with this bug. As far as I know, Flash team is small and they don't want to introduce any new feature.
Flags: needinfo?(amarchesini)
Please route any communications with the Flash team through me.
(In reply to Benjamin Smedberg [:bsmedberg] from comment #2) > There is an existing NPAPI signal that tells Flash that it is invisible and > should stop painting (we send it a 0x0 window). So we can only stop painting when the tab is invisible, but the video's timeclock would still go forward? Does this code would be ran everytime when tab is invisible? Could you help me point out where is the code? Thanks!
Flags: needinfo?(ehsan) → needinfo?(benjamin)
(In reply to Andrea Marchesini [:baku] from comment #3) > I think Benjamin gave a perfect description of what we should do for this > bug. Basically, you should contact somebody from the Flash team in adobe and > ask if they want to introduce this feature. If yes, we can continue with > this bug. > As far as I know, Flash team is small and they don't want to introduce any > new feature. Thanks for the information! If implement this feature needs to spend lots time and resource (I mean we should also need the help from Flash team), we should re-consider whether we really need this one, or we can just use mute to replace it. Because the final goal of us is that we want users using HTML5 video, instead of Flash. --- NI bwu and rachelle for putting this issue on their radar.
Flags: needinfo?(ryang)
Flags: needinfo?(bwu)
See bug 583109 for when we originally added visibility notifications.
Flags: needinfo?(benjamin)
Hi, Benjamin, I'm wondering why Chrome and Safari can pause the Flash even Flash doesn't support "pause", so I tried to open new video in background tab using Chrome and Safari in this website [1], and I found that they seem to postpone the loading of Flash until the tab becomes visible, instead of pausing it. Is my assumption right? If so, do you know how to achieve that? Thanks! [1] https://goo.gl/dMktXC
Flags: needinfo?(benjamin)
I've tried to postpone calling nsNPAPIPluginInstance::Start() before the tab becomes visible, but it seems wrong, because I can't receive "NPPVpluginIsPlayingAudio" after recalling start() when tab is visible.
Because Chrome uses PPAPI instead of NPAPI, it may be that they can forcefully pause flash instances. It may also be that they delay starting Flash instances until some later point in time. We might be able to delay starting Flash instances, although the possibility of causing regressions is rather high. I don't understand comment 9.
Flags: needinfo?(benjamin)
Alastor asked me a very good question that should we not support the websites which still use Flash? I am with him and we should not spend our resource in fixing the bugs related to Flash. Besides, we should encourage those websites to stop using Flash.
Flags: needinfo?(bwu)
Hi Blake and Alastor, Thank you for your efforts and sharing of the support status of block media autoplay (comment11)
Flags: needinfo?(ryang)
(In reply to Blake Wu [:bwu][:blakewu] from comment #11) > Alastor asked me a very good question that should we not support the > websites which still use Flash? > I am with him and we should not spend our resource in fixing the bugs > related to Flash. > Besides, we should encourage those websites to stop using Flash. Anthony, May I pick your brain on this? :-)
Flags: needinfo?(ajones)
We are working towards making Flash click-to-activate by default, tenatively in 55. Any outcome here has a pretty long success timeframe (well beyond Nov) so I'm going to make a high-level decision and WONTFIX a generic NPAPI pause API.
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(ajones)
Resolution: --- → WONTFIX
According to the comment14, we should give up requesting this new feature to Adobe. So I would like to change this bug for another possible option which is to postpone loading Flash library [1] until the tab is resumed. If we can do that, it's enough to prevent Flash playing in background blocked tab. --- But it needs sometime for me to figure out all related stuff about loading the Flash library. [1] https://goo.gl/ldeQA8
Assignee: nobody → alwu
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Summary: Support "pause" for NPAPI Flash → Postpone loading NPAPI Flash library until the tab was resumed
Status: REOPENED → NEW
Summary: Postpone loading NPAPI Flash library until the tab was resumed → Postpone loading NPAPI Flash library until the tab is resumed
Draft version, but it would crash after CC, I'm still debugging.
Fix the problem : showing 'play tab' icon even when users don't allow to activate plugin. Now I'm modifying test cases.
Summary: Postpone loading NPAPI Flash library until the tab is resumed → Postpone loading NPAPI Flash content until the tab is resumed
Attachment #8856493 - Flags: review?(benjamin)
Comment on attachment 8856493 [details] Bug 1350947 - start loading plugin content after the tab is resumed. https://reviewboard.mozilla.org/r/128354/#review134070 This is a *very* risky change, and I'm not convinced that the benefits here are worth the risks. As I understand it, this would make any tab that was loaded in the background and had Flash content show the paused-media indicator. That is already risky because most Flash is not for media and this could be considered a false positive result. Secondly, delaying the load of Flash content is going to break some kinds of JS. Web JS assumes that Flash is instantiated synchronously, and that the proto chain of a plugin element will be available at certain sync points; my understanding is that this patch doesn't delay the "load" event for example, which would pretty much guarantee major breakage. For some examples of how we've tried (and failed) to do this in th So I need you to convince me that this patch is going to be worth the overall risk. And then qdot would be the final reviewer, since this is his DOM code and not plugin code.
Attachment #8856493 - Flags: review?(benjamin) → review-
(In reply to Benjamin Smedberg [:bsmedberg] from comment #28) > As I understand it, this would make any tab that was loaded in the > background and had Flash content show the paused-media indicator. That is > already risky because most Flash is not for media and this could be > considered a false positive result. As I know, the <embed> and <obj> is autoplay by default, so yes, the interface might be wrong when it's blocked. But why it's risky? Could you give me any examples? User won't be aware of that because Flash would be resumed after the tab becomes foreground. > Secondly, delaying the load of Flash content is going to break some kinds of > JS. Web JS assumes that Flash is instantiated synchronously, and that the > proto chain of a plugin element will be available at certain sync points; my > understanding is that this patch doesn't delay the "load" event for example, > which would pretty much guarantee major breakage. I can also delay it if you want. However, I didn't see the "load" event in <embed> and <object> [1]. Where does this event come from? [1] http://searchfox.org/mozilla-central/source/dom/webidl/HTMLObjectElement.webidl http://searchfox.org/mozilla-central/source/dom/webidl/HTMLEmbedElement.webidl > For some examples of how we've tried (and failed) to do this in th Missing some words? Thanks!
Flags: needinfo?(benjamin)
> give me any examples? User won't be aware of that because Flash would be > resumed after the tab becomes foreground. The page scripts assume the synchronous setup before the page 'load' event, and then break. So it can be unrecoverable for the user without a reload. See bug 1195607 for some of the compat issues that came out of async plugin init. > I can also delay it if you want. However, I didn't see the "load" event in > <embed> and <object> [1]. > Where does this event come from? The load event on the page itself. Although I thought object elements had a load event also, since they are pseudo-iframes. That wouldn't be in the webidl, as it's an event dispatch. > > > For some examples of how we've tried (and failed) to do this in th Argh, see the whole history of async plugin init. http://dblohm7.ca/blog/2017/04/07/asynchronous-plugin-initialization-requiem/ and bug 998863 and bug 1116806.
Flags: needinfo?(benjamin)
Hi, Benjamin, If I understand correctly, what you did for async initialization includes creating plugin instance, plugin host, create remote process and other complex stuffs. So, if I only postpone loading the URL, I don't modify other setup. The instance, host and other things should work as expectedly, would that still cause any problem? As I know, even JS receive the document's "load" event, it doesn't mean the element has loaded successfully, right? If so, we still might have the situation that the URL can't be loaded correctly, eg. if the network state is not stable. Please correct me if my understand is incorrect! Thanks!
Flags: needinfo?(benjamin)
(In reply to Alastor Wu [:alwu][please needinfo? me] from comment #31) > So, if I only postpone loading the URL, I don't modify other setup. The > instance, host and other things should work as expectedly, would that still > cause any problem? > What does "postpone loading the URL" mean in practice? Does this also postpone firing the onload event to the page? > As I know, even JS receive the document's "load" event, it doesn't mean the > element has loaded successfully, right? Assuming that there weren't timeouts or 404s then yes, the load event means that all the page elements have loaded. The page onload handler typically assumes that the plugin is scriptable in expected ways, and will break if it's not.
Flags: needinfo?(benjamin)
(In reply to Benjamin Smedberg [:bsmedberg] from comment #32) > What does "postpone loading the URL" mean in practice? Does this also > postpone firing the onload event to the page? Sorry, I misunderstood the window's onload event, I thought window "onload" event didn't have direct relationship with plugin's loading progress, but it's wrong. So yes, I can call |document.blockonload()| to postpone the window's load event until the page was resumed and we started load the URL for plugin. How do you think? Thanks!
Flags: needinfo?(benjamin)
Comment on attachment 8856493 [details] Bug 1350947 - start loading plugin content after the tab is resumed. https://reviewboard.mozilla.org/r/128354/#review141694 So I'm absolutely not going to review this without fairly comprehensive automated tests. I know this is going to cause some regressions, but I don't know how bad they will be. It doesn't appear that this is pref-controlled, which is something we need to mitigate the risks. How have you tested this, and have you been testing this on topsites such as facebook games and the popular video sites that still use Flash? As an example of something that would break: https://github.com/swfobject/swfobject/blob/master/swfobject/src/swfobject.js#L188 the `testPlayerVersion` function inserts a Flash element and synchronously scripts it to get the Flash $version variable. SWFObject is widely used across the web. I'm still skeptical of the reward provided by this patch once we have click-to-play, which is landing this cycle.
Attachment #8856493 - Flags: review?(benjamin) → review-
Comment on attachment 8856493 [details] Bug 1350947 - start loading plugin content after the tab is resumed. https://reviewboard.mozilla.org/r/128354/#review141704 I'm on board with bsmedberg here. Tests and proof this will help on top of ctp are really gonna be needed for something that could possible affect content like this.
Attachment #8856493 - Flags: review?(kyle) → review-
Flags: needinfo?(benjamin)
(In reply to Benjamin Smedberg [:bsmedberg] from comment #36) > Comment on attachment 8856493 [details] > Bug 1350947 - start loading plugin content after the tab is resumed. > > https://reviewboard.mozilla.org/r/128354/#review141694 > > So I'm absolutely not going to review this without fairly comprehensive > automated tests. Could you help me to tell what kind of test should I add for this? Now I have one to test for checking whether the playback can be resumed successfully after visiting the tab [1] Since I don't familiar how would user interact with plug-in, I have no idea what are other situations I should test for. [1] http://searchfox.org/mozilla-central/source/toolkit/content/tests/browser/browser_block_plugIn.js > I know this is going to cause some regressions, but I don't know how bad > they will be. It doesn't appear that this is pref-controlled, which is > something we need to mitigate the risks. How have you tested this, and have > you been testing this on topsites such as facebook games and the popular > video sites that still use Flash? I test on websites which are reported by bug1348759, bug1349202 and bug1348879, and all of them are good. Also, the facebook games work well. > As an example of something that would break: > https://github.com/swfobject/swfobject/blob/master/swfobject/src/swfobject. > js#L188 the `testPlayerVersion` function inserts a Flash element and > synchronously scripts it to get the Flash $version variable. SWFObject is > widely used across the web. Why this code would be broke? I just postpone to load the URL, doesn't stop to activate other plugin stuffs. I wrote the test which has similar behavior [2] and we still can get the correct version even when the tab is blocked. [2] https://people-mozilla.org/~alwu/flashTest2.html (If you want, I can add it as a test) > I'm still skeptical of the reward provided by this patch once we have > click-to-play, which is landing this cycle. That is two different things, since "click-to-play" can only prevent autoplay media from the website which the users doesn't allow them to play flash by default. But, for the websites user usually visits, user might allow the flash can always be playback. In this case, the Flash media content would work inconsistently with other HTML5 media content, because it would still playback in background. Thanks!
Flags: needinfo?(benjamin)
> Could you help me to tell what kind of test should I add for this? You add a test for a site that loads a plugin and test whether the plugin is paused properly by this feature you're developing. > I test on websites which are reported by bug1348759, bug1349202 and > bug1348879, and all of them are good. Also, the facebook games work well. I'm willing to accept that this feature works most of the time as intended (although automated tests are still required). I'm much more concerned about the cases where it *doesn't* work. The risk profile of any change to plugins is huge because we often don't find regressions until release. > I just postpone to load the URL, doesn't stop to activate other plugin > stuffs. > I wrote the test which has similar behavior [2] and we still can get the > correct version even when the tab is blocked. > > [2] https://people-mozilla.org/~alwu/flashTest2.html So you're saying that with this patch, you still synchronously instantiate the plugin, but you delay loading the SWF file? That does reduce the risk, although it's hard to say by how much. > But, for the websites user usually visits, user might allow the flash can > always be playback. In this case, the Flash media content would work > inconsistently with other HTML5 media content, because it would still > playback in background. Sure, but it affects the overall impact of the feature. > Do you know whether we could use those API [1] to pause media in Gecko C++ > side? No, I don't know whether those would be reliable or not.
Flags: needinfo?(benjamin)
(In reply to Benjamin Smedberg [:bsmedberg] from comment #40) > You add a test for a site that loads a plugin and test whether the plugin is > paused properly by this feature you're developing. Could I enable my own Flash content by default in the test? Or I could only use the type "application/x-shockwave-flash-test"? The reason I don't want to use the "application/x-shockwave-flash-test" is that I would like to use some real swf file and check whether it can be resumed successfully. Now I'm writing test and want to automatically active the permission for the embed tag with src="xxx.swf" in my test, but it always show the window which ask me to "click-to-play". > I'm willing to accept that this feature works most of the time as intended > (although automated tests are still required). I'm much more concerned about > the cases where it *doesn't* work. The risk profile of any change to plugins > is huge because we often don't find regressions until release. Yes, I agree, so if you figure out any possible broken situation, please tell me and I'll write the test. > So you're saying that with this patch, you still synchronously instantiate > the plugin, but you delay loading the SWF file? Yes, the instantiation would still be synchronous. Thanks!
Flags: needinfo?(benjamin)
> Could I enable my own Flash content by default in the test? Or I could only > use the type "application/x-shockwave-flash-test"? The Flash plugin is not installed on our test slaves, so you normally need to construct automated tests using the test plugin. > Now I'm writing test and want to automatically active the permission for the > embed tag with src="xxx.swf" in my test, but it always show the window which > ask me to "click-to-play". Plugin test files have to explicitly set the activation settings to the expected state. > Yes, I agree, so if you figure out any possible broken situation, please > tell me and I'll write the test. It's your job to make sure that your test plan matches up to the risk of this feature. Automated testing is definitely not going to be sufficient to ensure that this doesn't break other web content. > Yes, the instantiation would still be synchronous. Kyle and Aaron, what is your advice/opinion about how this changes the risk profile?
Flags: needinfo?(kyle)
Flags: needinfo?(benjamin)
Flags: needinfo?(aklotz)
(In reply to Benjamin Smedberg [:bsmedberg] from comment #42) > The Flash plugin is not installed on our test slaves, so you normally need > to construct automated tests using the test plugin. > I mean, I don't want to use the type "application/x-shockwave-flash-test", I would like to load real swf file in <embed>, like that ``` <embed src="audibleSWF.swf" width="200" height="200" type="application/x-shockwave-flash"> ``` But when I did that, the browser always show the window which ask me to "click-to-play". > Plugin test files have to explicitly set the activation settings to the > expected state. As above, I would like to use an <embed> which doesn't have permission yet. I would like to know how to get the permission correctly. > It's your job to make sure that your test plan matches up to the risk of > this feature. Automated testing is definitely not going to be sufficient to > ensure that this doesn't break other web content. I already have enough test cases can cover the media element and web audio, but I'm not the expert of plug-in and I don't know how and what you guys would use plug-in on the web, that is why I need your help here. Now the only test I can do is like above, to test the <embed> with loading swf file and check whether it can be resumed successfully. Thanks!
Flags: needinfo?(benjamin)
(In reply to Benjamin Smedberg [:bsmedberg] from comment #42) > Kyle and Aaron, what is your advice/opinion about how this changes the risk > profile? Prior to answering, thanks for all your help. If all of your guys think it's too risky to change the plug-in's behavior (only postpone loading content, but the instantiation would still go on), I would discuss this issue with my manager and PM again.
(In reply to Alastor Wu [:alwu][please needinfo? me] from comment #43) > I mean, I don't want to use the type "application/x-shockwave-flash-test", I > would like to load real swf file in <embed>, like that > > ``` > <embed src="audibleSWF.swf" width="200" height="200" > type="application/x-shockwave-flash"> > ``` > > But when I did that, the browser always show the window which ask me to > "click-to-play". You can't use x-shockwave-flash in tests at all, because on the test slaves the Flash plugin is not installed. You have to develop your test using the test plugin in order for it to run in automation. If it *were* installed, you'd still have to explicitly change it from ask-to-activate to always-activate. There are plenty of examples of doing this in the existing plugin tests. > but I'm not the expert of plug-in and I don't know how and what you guys > would use plug-in on the web, that is why I need your help here. I'm sorry, the plugin team is me and I do not have the time to focus on the testing for this problem. I need you to read the prior history and work with a QA team to develop a test plan that covers the risks here.
Flags: needinfo?(benjamin)
This does not sound ready to land in 55, whatever we end up doing later. What sites do we know are affected? Should this be a priority?
Making this synchronous clears up some problems, though we'll also need to make sure this doesn't wedge any telemetry, otherwise we're going to start getting really weird load time numbers as things wait for a tab to become active. Honestly, I'd like to know how often we're seeing loads in the background so we'd know what kind of impact this might have, but I guess the time for that kind of research has passed. Overall, I still think this is a really risky change with all of the other plugin fixes we've already got in the pipeline, and I'm worried about how much maintenance it would mean in the future.
Flags: needinfo?(kyle)
Yeah, as cool as this is, I've got to agree that this is up there with async plugin init in terms of risk level. It's the second-order ignorance (ie, the "unknown unknowns") that I'd be worried about.
Flags: needinfo?(aklotz)
Cindy, This is what I mentioned in our meeting that block-auto-playback has different behavior between the video played via HTML5 video tag and Flash. For a video played via HTML5, we will pause it but for Flash case we mute. Alastor has done a great job in trying to fix this flash problem but it looks the risk is too high and it would be not worth doing it since HTML5 has been used more and more for video playback and Flash hasn't. Ni you to let you know the context and see if you have any concerns.
Flags: needinfo?(chsiang)
Hi Blake, Thanks for the message. I'm less concerned about blocking flash videos but communicating on-tab media buttons clearly and consistently to users. We can discuss more as we walk through the UX specs tomorrow. Thanks. Cindy
Flags: needinfo?(chsiang)
After discussed with bwu, we decide to stop working on this bug because of its high risk. (comment47, comment48) Feel free to reopen it if you have better solution.
Status: NEW → RESOLVED
Closed: 8 years ago7 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: