Fullscreen Flash video does not appear on ICS

VERIFIED FIXED in Firefox 14

Status

()

Firefox for Android
General
P1
normal
VERIFIED FIXED
5 years ago
a year ago

People

(Reporter: snorp, Assigned: snorp)

Tracking

({productwanted})

Trunk
Firefox 16
ARM
Android
productwanted
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox14 verified, firefox15 fixed, firefox16 verified, blocking-fennec1.0 soft, fennec14+)

Details

(Whiteboard: l10n)

Attachments

(1 attachment, 2 obsolete attachments)

On sites like YouTube, only the player controls and other non-video content appears in fullscreen mode.
blocking-fennec1.0: --- → ?
Blocks: 727421
blocking-fennec1.0: ? → +
Keywords: productwanted
Provisionally plussing, but we need product's call on whether this is a blocker.
Although this would be good to have in our first release, we don't yet have a fix properly identified and I'm not sure there's enough time to 1) identify a fix, 2) write the fix & implement it, and 3) test it appropriately for launch. 

Feedback in the reviews on the topic have requested support, but so far no one has truly complained about the lack of (at least yet).

Therefore to mitigate, can we please add a string that layers over the box stating that full screen is not currently supported for the operating system (this should obviously only appear for those on ICS!). This will mitigate the issue of users just not knowing why it's not working.

We need to get this issue fixed for the next release, though. We'll add this as a key discussion in the release stakeholders meeting June 5 to ensure everyone has been looped in.
Whiteboard: l10n

Comment 3

5 years ago
I'll allow it, for:

- well separated UI element
- the users life sucks at that point anyway, and English string is better than failure
tracking-fennec: --- → 14+
blocking-fennec1.0: + → soft

Updated

5 years ago
Blocks: 761773
Created attachment 630660 [details] [diff] [review]
Fix up fullscreen Flash handling on Android 4.0+
Attachment #630660 - Flags: review?(blassey.bugs)
Comment on attachment 630660 [details] [diff] [review]
Fix up fullscreen Flash handling on Android 4.0+

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

Great work. The postDelayed()'s make me worry about race conditions they may be band-aiding.

::: mobile/android/base/GeckoApp.java
@@ +1579,5 @@
> +        mMainHandler.postDelayed(new Runnable() { 
> +            public void run() {
> +                mLayerController.getView().setVisibility(View.VISIBLE);
> +            }
> +        }, 500);

why post delayed? Changed this to just post() locally and it works fine.

@@ +3134,5 @@
> +            mMainHandler.postDelayed(new Runnable() { 
> +                public void run() {
> +                    mLayerController.getView().setVisibility(View.INVISIBLE);
> +                }
> +            }, 500);

again I'm questioning the postDelayed(). This is working for me locally:
            super.addView(view, index);
            mMainHandler.post(new Runnable() { 
                public void run() {
                    mLayerController.getView().setVisibility(View.INVISIBLE);
                }
            });

@@ +3145,5 @@
> +            if (event.isSystem()) {
> +                return super.onKeyDown(keyCode, event);
> +            }
> +            mFullScreenPluginView.onKeyDown(keyCode, event);
> +            return true;

should be:
return mFullScreenPluginView.onKeyDown(keyCode, event);

no?

@@ +3154,5 @@
> +            if (event.isSystem()) {
> +                return super.onKeyUp(keyCode, event);
> +            }
> +            mFullScreenPluginView.onKeyUp(keyCode, event);
> +            return true;

same as above

@@ +3159,5 @@
> +        }
> +
> +        @Override
> +        public boolean onTouchEvent(MotionEvent event) {
> +            return true;

if we don't pass these to the plugin, I think we should return false instead of true.

@@ +3165,5 @@
> +
> +        @Override
> +        public boolean onTrackballEvent(MotionEvent event) {
> +            mFullScreenPluginView.onTrackballEvent(event);
> +            return true;

same as above
Attachment #630660 - Flags: review?(blassey.bugs) → review-
Created attachment 630809 [details] [diff] [review]
patch with nits addressed
Attachment #630809 - Flags: review?(snorp)
Created attachment 630813 [details] [diff] [review]
Fix up fullscreen Flash handling on Android 4.0+
Attachment #630813 - Flags: review?(blassey.bugs)
Attachment #630660 - Attachment is obsolete: true
Attachment #630809 - Attachment is obsolete: true
Attachment #630809 - Flags: review?(snorp)
Comment on attachment 630813 [details] [diff] [review]
Fix up fullscreen Flash handling on Android 4.0+

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

::: mobile/android/base/GeckoApp.java
@@ +3131,5 @@
> +             * it for some reason so we need to hide that. Hiding the LayerView causes
> +             * its surface to be destroyed, which causes a pause composition
> +             * event to be sent to Gecko. We synchronously wait for that to be
> +             * processed. Simultaneously, however, Flash is waiting on a mutex so
> +             * the delay below is an attempt to avoid a deadlock.

comment needs to be reworked
Attachment #630813 - Flags: review?(blassey.bugs) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d1aabb06e7c
https://hg.mozilla.org/mozilla-central/rev/1d1aabb06e7c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
Issue resolved on a glance tour through a latest inbound; (06/07); YouTube full-screen is working for me on my Galaxy Nexus (4.0.4), and Galaxy Note (4.0.3). Marking status-firefox16 as fixed.

This still a softy?
Status: RESOLVED → VERIFIED
status-firefox16: --- → verified
https://hg.mozilla.org/releases/mozilla-aurora/rev/c9aa071bf2fd
https://hg.mozilla.org/releases/mozilla-beta/rev/b0f32cd82c97
https://hg.mozilla.org/releases/mozilla-beta/rev/33043ace32f0
status-firefox14: --- → fixed
status-firefox15: --- → fixed
Duplicate of this bug: 764635
Duplicate of this bug: 764657
Full screen works... non full screen doesn't show video right always on ics ?

Need to investigate more.  Marking verified for full screen flash video for 14 b7 candidate 3.
status-firefox14: fixed → verified
You need to log in before you can comment on or make changes to this bug.