Closed Bug 701785 Opened 10 years ago Closed 9 years ago

Missing html5 video/audio controls in context menu

Categories

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

ARM
Android
defect

Tracking

(firefox16 affected, fennec19+)

VERIFIED FIXED
Firefox 20
Tracking Status
firefox16 --- affected
fennec 19+ ---

People

(Reporter: tchung, Assigned: mfinkle)

References

()

Details

(Whiteboard: [testday-20111111][qa^])

Attachments

(1 file, 1 obsolete file)

There seems to be a handful of missing context menu items for HTML5 video window.

- Play/Pause
- Save Video
- Share Video

Only Full screen is there.   These are available on XUL Fennec

Repro:
1) install Fennec Native nightly, 20111111 build
2) visit URL or a site that has HTML5 video samples
3) play video via webpage controls
4) context click a video.  verify missing context menu options listed above

Expected:
- main video controls appear in context click menu

ActuaL:
- only full screen is an option for video controls
Wes, dupe of 699537?
Summary: Missing html5 video controls in context menu → Missing html5 video/audio controls in context menu
Yeah they're listed in bug 699537.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 699537
Status: RESOLVED → VERIFIED
reopening, for ease of tracking context menu work (via wesj)

adding blocking bug 699537
Blocks: 699537
Status: VERIFIED → REOPENED
Resolution: DUPLICATE → ---
Priority: -- → P4
Sriram - we have code in XUL fennec that shows how to implement these menu items.
Assignee: nobody → sriram
Whiteboard: [testday-20111111] → [testday-20111111][qa^]
The context menu options are missing, except Full Screen if a long tap is performed over a HTML5 video.

--
Firefox 16.0b1 (2012-08-28)
Device: Galaxy Note
OS: Android 4.0.4
tracking-fennec: --- → ?
Looks like Sriram hasn't been working on this, but it would make a good mentor bug, so I'm marking it as such.
Assignee: sriram → nobody
Whiteboard: [testday-20111111][qa^] → [testday-20111111][qa^][mentor=margaret][lang=js]
Status: REOPENED → NEW
tracking-fennec: ? → -
Duplicate of this bug: 804304
Renominating based on that we are getting more <video> support. Without this bug fixed users cannot save video/audio/
tracking-fennec: - → ?
Assignee: nobody → wjohnston
tracking-fennec: ? → 19+
Assignee: wjohnston → mark.finkle
Attached patch patch (obsolete) — Splinter Review
This patch adds: "Play", "Pause" and "Share Video" to the context menu. Both "Play" and "Pause" should appear for video and audio. "Share Video" only works for video.

"Play" and "Pause" are exclusive, so only one is visible at a time, depending on the state of the media element.

I tested this change on webm and ogv, both inline and fullscreen. Seems to work OK.
Attachment #684297 - Flags: review?(wjohnston)
Comment on attachment 684297 [details] [diff] [review]
patch

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

::: mobile/android/chrome/content/browser.js
@@ +1463,5 @@
> +            if (paused && aMode == "media-paused")
> +              return true;
> +            if (!paused && aMode == "media-playing")
> +              return true;
> +          }

Desktop also checks for load errors here:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#395

Probably should do the same thing here.
Attachment #684297 - Flags: review?(wjohnston) → review-
Attached patch patch 2Splinter Review
Added the "hasError" check.
Attachment #684297 - Attachment is obsolete: true
Attachment #685877 - Flags: review?(wjohnston)
Comment on attachment 685877 [details] [diff] [review]
patch 2

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

Seems good to me!

The desktop implementation just sets these items disabled. We actually support that, but I don't think there's anything in the context menu api for it, and it seems like a waste of space on small screens.
Attachment #685877 - Flags: review?(wjohnston) → review+
(In reply to Wesley Johnston (:wesj) from comment #12)

> The desktop implementation just sets these items disabled. We actually
> support that, but I don't think there's anything in the context menu api for
> it, and it seems like a waste of space on small screens.

Right. I saw no reason to show them disabled.
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc0d354ed028
Whiteboard: [testday-20111111][qa^][mentor=margaret][lang=js] → [testday-20111111][qa^]
Target Milestone: --- → Firefox 20
https://hg.mozilla.org/mozilla-central/rev/bc0d354ed028
Status: NEW → RESOLVED
Closed: 10 years ago9 years ago
Resolution: --- → FIXED
I cannot reproduce this issue on the latest Nightly and Aurora. Closing bug as verified fixed on:
Firefox for Android
Version: 21.0a1 (2013-02-10)
Device: HTC Desire Z
OS: Android 2.3.3
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.