727421 - Full screen does not work correctly on youtube.com

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Camelia Urian]

Attachment: 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 (dead)]

On ICS, this seems to make Fennec unusable.

Comment 2

[Erin Lancaster [:elan]]

Keeping on the blocker list; Damon + JP have the action to follow up with Adobe.

Comment 3

[Jet Villegas (inactive)]

Assigning to self to get more info.

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

I've figured out what was causing most of the wonky behavior here. Taking this one back.

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Implement full screen support for Flash on Android

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

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?)]

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) ([e-mail]) (he/him)]

Attachment: Implement full screen support for Flash on Android

Comment 11

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/814d564578d1

Comment 12

[James Willcox (:snorp) ([e-mail]) (he/him)]

This got backed out due to xul breakage. Working on a fix.

Comment 13

[James Willcox (:snorp) ([e-mail]) (he/him)]

Relanded

https://hg.mozilla.org/integration/mozilla-inbound/rev/2910fc77173d

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2910fc77173d

Comment 15

[Aaron Train [:aaronmt]]

Does not work at all on ICS.

Comment 16

[Kevin Brosnan [Ex-Mozilla]]

This is expected. Bug 759747

Comment 17

[James Willcox (:snorp) ([e-mail]) (he/him)]

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?)]

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) ([e-mail]) (he/him)]

https://hg.mozilla.org/releases/mozilla-beta/rev/0bf3896da2e7

Comment 20

[Catalin Suciu [:csuciu]]

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)