Last Comment Bug 727421 - Full screen does not work correctly on youtube.com
: Full screen does not work correctly on youtube.com
Status: VERIFIED FIXED
[QA^]
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: Trunk
: ARM Android
: P1 normal with 1 vote (vote)
: Firefox 15
Assigned To: James Willcox (:snorp) (jwillcox@mozilla.com)
:
Mentors:
http://www.youtube.com
: 753124 757499 (view as bug list)
Depends on: 759747 761773
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-15 06:15 PST by Camelia Urian
Modified: 2012-06-14 05:19 PDT (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
verified
betaN+


Attachments
Screenshot after full screen was selected (163.33 KB, image/png)
2012-02-15 06:15 PST, Camelia Urian
no flags Details
Implement full screen support for Flash on Android (40.15 KB, patch)
2012-05-22 11:43 PDT, James Willcox (:snorp) (jwillcox@mozilla.com)
blassey.bugs: review+
Details | Diff | Review
Implement full screen support for Flash on Android (43.09 KB, patch)
2012-05-29 12:19 PDT, James Willcox (:snorp) (jwillcox@mozilla.com)
blassey.bugs: review+
blassey.bugs: approval‑mozilla‑beta+
Details | Diff | Review

Description Camelia Urian 2012-02-15 06:15:51 PST
Created attachment 597382 [details]
Screenshot after full screen was selected

Fennec/13.0a1 2012-02-14
Device: HTC Desire (Android 2.2)

Steps to reproduce:
1. Make sure the Flash Plugin is installed.
2. Go to youtube.com and open any video.
3. Tap the full screen video control.
4. Try and scroll the page.

Expected results:
Video plays in full screen.

Actual results:
At step 3 the video is never set to full screen.
At step 4 if the page is scrolled the videoplayer is repainted but it is painted only a fraction of the screen size. 

Notes:
The issue is not reproducible on the Android Browser.
Comment 1 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2012-03-02 10:06:13 PST
On ICS, this seems to make Fennec unusable.
Comment 2 Erin Lancaster [:elan] 2012-04-16 12:07:07 PDT
Keeping on the blocker list; Damon + JP have the action to follow up with Adobe.
Comment 3 Jet Villegas (:jet) 2012-05-01 12:33:09 PDT
Assigning to self to get more info.
Comment 4 James Willcox (:snorp) (jwillcox@mozilla.com) 2012-05-08 13:12:50 PDT
I've figured out what was causing most of the wonky behavior here. Taking this one back.
Comment 5 Aaron Train [:aaronmt] 2012-05-08 16:56:31 PDT
*** Bug 753124 has been marked as a duplicate of this bug. ***
Comment 6 James Willcox (:snorp) (jwillcox@mozilla.com) 2012-05-22 10:19:48 PDT
*** Bug 757499 has been marked as a duplicate of this bug. ***
Comment 7 James Willcox (:snorp) (jwillcox@mozilla.com) 2012-05-22 11:43:29 PDT
Created attachment 626109 [details] [diff] [review]
Implement full screen support for Flash on Android
Comment 8 James Willcox (:snorp) (jwillcox@mozilla.com) 2012-05-22 11:44:28 PDT
The above patch works fine on Gingerbread, but has a problem on ICS; the video content is not painted. I've been trying to figure that one out for a while now, but this is at least better than what we do now (which is deadlock the browser).
Comment 9 Brad Lassey [:blassey] (use needinfo?) 2012-05-24 18:01:52 PDT
Comment on attachment 626109 [details] [diff] [review]
Implement full screen support for Flash on Android

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

::: dom/plugins/base/android/ANPSurface.cpp
@@ +165,5 @@
>    LOG("Initialized? %d\n", gSurfaceFunctions.initialized);
>    return gSurfaceFunctions.initialized;
>  }
>  
> +// FIXME: All of this should be changed to use the equivalent things in AndroidBridge

file that bug

::: dom/plugins/base/android/ANPWindow.cpp
@@ +125,5 @@
> +    case kLandscape_ANPScreenOrientation:
> +      newOrientation = 6;
> +      break;
> +    case kPortrait_ANPScreenOrientation:
> +      newOrientation = 7;

can we define these magic numbers somewhere?

::: dom/plugins/base/nsNPAPIPluginInstance.cpp
@@ +73,5 @@
>  #ifdef MOZ_WIDGET_ANDROID
>      mSurface(nsnull),
>      mANPDrawingModel(0),
>      mOnScreen(true),
> +    mFullScreenOrientation(0 /* fixed landscape, see ActivityInfo class */),

again, no magic numbers

::: dom/plugins/base/nsPluginInstanceOwner.cpp
@@ +1743,4 @@
>    }
>  
>    if (AndroidBridge::Bridge())
> +    AndroidBridge::Bridge()->AddPluginView((jobject)mJavaView, aRect, mFullScreen, mInstance->FullScreenOrientation());

you need a jniframe and to check for an exception after this

@@ +1759,2 @@
>    if (AndroidBridge::Bridge())
> +    AndroidBridge::Bridge()->RemovePluginView((jobject)mJavaView, mFullScreen);

jniframe and exception check needed
Comment 10 James Willcox (:snorp) (jwillcox@mozilla.com) 2012-05-29 12:19:48 PDT
Created attachment 628054 [details] [diff] [review]
Implement full screen support for Flash on Android
Comment 11 James Willcox (:snorp) (jwillcox@mozilla.com) 2012-05-30 07:29:59 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/814d564578d1
Comment 12 James Willcox (:snorp) (jwillcox@mozilla.com) 2012-05-30 09:07:53 PDT
This got backed out due to xul breakage. Working on a fix.
Comment 13 James Willcox (:snorp) (jwillcox@mozilla.com) 2012-05-30 09:11:31 PDT
Relanded

https://hg.mozilla.org/integration/mozilla-inbound/rev/2910fc77173d
Comment 14 Ed Morley [:emorley] 2012-05-31 06:34:38 PDT
https://hg.mozilla.org/mozilla-central/rev/2910fc77173d
Comment 15 Aaron Train [:aaronmt] 2012-05-31 09:51:44 PDT
Does not work at all on ICS.
Comment 16 Kevin Brosnan [:kbrosnan] 2012-05-31 10:00:05 PDT
This is expected. Bug 759747
Comment 17 James Willcox (:snorp) (jwillcox@mozilla.com) 2012-06-04 18:39:07 PDT
Comment on attachment 628054 [details] [diff] [review]
Implement full screen support for Flash on Android

[Approval Request Comment]
User impact if declined: After pressing fullscreen buttons on Flash plugins, browser becomes unusable
Testing completed (on m-c, etc.): m-c for a couple weeks
Risk to taking this patch (and alternatives if risky): Bug 759747
String or UUID changes made by this patch: None
Comment 18 Brad Lassey [:blassey] (use needinfo?) 2012-06-05 12:03:19 PDT
Comment on attachment 628054 [details] [diff] [review]
Implement full screen support for Flash on Android

[Triage Comment]
approved for beta (assuming that this is already on aurora)
Comment 19 James Willcox (:snorp) (jwillcox@mozilla.com) 2012-06-05 16:39:23 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/0bf3896da2e7
Comment 20 Catalin Suciu [:csuciu] 2012-06-14 05:19:36 PDT
Unable to reproduce on:
Nightly 16.0a1 (2012-06-14)
Aurora 15.0a2 (2012-06-14)
Beta 14.0b7 Build 2

Samsung Galaxy SII (2.3.4)

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