Last Comment Bug 715730 - Flash app restarts after clicking a selection when click to play is turned on
: Flash app restarts after clicking a selection when click to play is turned on
Status: VERIFIED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: 12 Branch
: ARM Android
: -- normal (vote)
: ---
Assigned To: :Margaret Leibovic
:
Mentors:
https://settings.adobe.com
: 713080 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-05 16:46 PST by Naoki Hirata :nhirata (please use needinfo instead of cc)
Modified: 2016-07-29 14:21 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
logcat.txt (16.04 KB, text/plain)
2012-01-05 16:46 PST, Naoki Hirata :nhirata (please use needinfo instead of cc)
no flags Details
patch (4.57 KB, patch)
2012-01-06 12:03 PST, :Margaret Leibovic
mark.finkle: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-01-05 16:46:55 PST
Created attachment 586275 [details]
logcat.txt

1. go to menu -> more -> settings -> enable plugins -> Click to play
2. hit the back button and go to : https://settings.adobe.com
3. click on where it states "Tap here to activate plugins"
4. select Local Storage

Expected: flash will continue on with the app
Actual: flash app restarts

Note:
1. Samsung Nexus S, 2.3.1, flash 11, 20120105 build
2. also the flash plugin will continue to the next screen if yes was selected for the enable plugins option.
Comment 1 :Margaret Leibovic 2012-01-05 18:03:12 PST
This feels like a problem with the flash settings app. Possibly a dupe of bug 694546?
Comment 2 Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-01-05 18:37:38 PST
It's talking about the same portion, but I don't believe it's the same bug.  In the bug 694546 it's stating that it doesn't work at all.  I believe that it does work, but not when "Click to play" is enabled.
Comment 3 :Margaret Leibovic 2012-01-05 18:57:35 PST
With click to play enabled, I can tap to activate the plugin, but I run into problems when I try to select local storage like you say in comment 0. Does the app work correctly if you just have enable plugins set to "yes"?
Comment 4 Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-01-05 20:00:58 PST
Yes, the app seems to work correctly if I just enable the plugins to yes and then restart fennec.
Comment 5 :Margaret Leibovic 2012-01-06 08:19:09 PST
Okay, I'm seeing the same thing. I just discovered if I hit the back button after going to the busted local storage page, then click on local storage again it works. Maybe there's a problem with the order in which we're loading the plugins on the page?
Comment 6 Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-01-06 11:16:02 PST
Oh interesting.  Is there a way to test order of load easier?
Comment 7 :Margaret Leibovic 2012-01-06 11:40:32 PST
(In reply to Naoki Hirata :nhirata from comment #6)
> Oh interesting.  Is there a way to test order of load easier?

Actually, I think this problem is caused by the same root issue that's causing bug 713080 - we're re-loading all the plugins when the plugin objects are tapped, even after they're loaded. I'm working on a patch now that will hopefully fix both these bugs.
Comment 8 :Margaret Leibovic 2012-01-06 12:03:21 PST
Created attachment 586506 [details] [diff] [review]
patch

We were reloading the plugins every time the plugin object received a click event. Since we're playing all plugins when one is clicked, it's kind of tricky to remove the click event listeners, so I decided to just clear _pluginsToPlay and check its length when playAllPlugins gets called.

I also decided to get rid of the addPluginClickCallback I mostly stole from desktop, since we don't need that generic helper function, and I just replaced it with a normal click listener.

I think this will fix bug 713080 and bug 694546 as well. (I was hoping it would also fix bug 715740, but that doesn't seem to be the case.)
Comment 9 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-06 12:11:03 PST
Comment on attachment 586506 [details] [diff] [review]
patch

Test switching between tabs with Flash to make sure we don't lose plugin info
Comment 11 :Margaret Leibovic 2012-01-09 12:31:38 PST
*** Bug 713080 has been marked as a duplicate of this bug. ***
Comment 12 :Margaret Leibovic 2012-01-09 12:33:40 PST
This never got marked as fixed, but it made its way into mozilla-central:

https://hg.mozilla.org/mozilla-central/rev/5971546ca35e
Comment 13 :Margaret Leibovic 2012-01-09 12:36:33 PST
Comment on attachment 586506 [details] [diff] [review]
patch

[Approval Request Comment]
User impact if declined: interactive flash objects (like embedded youtube videos) won't work
Testing completed (on m-c, etc.): baked on m-c for a few days
Comment 14 Alex Keybl [:akeybl] 2012-01-09 14:55:29 PST
Comment on attachment 586506 [details] [diff] [review]
patch

[Triage Comment]
Mobile only - approved for Aurora.
Comment 16 Cristian Nicolae (:xti) 2012-01-31 07:53:19 PST
Verified fixed on:

Mozilla/5.0 (Android;Linux armv7l;rv:11.0a2)Gecko/20120130
Firefox/11.0a2 Fennec/11.0a2
Device: Samsung Galaxy S
OS: Android 2.2

Mozilla/5.0 (Android;Linux armv7l;rv:12.0a1)Gecko/20120130
Firefox/12.0a1 Fennec/12.0a1
Device: Samsung Galaxy S
OS: Android 2.2

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