Closed Bug 556563 Opened 10 years ago Closed 8 years ago

Disable/remove "View video" when you're already viewing the video

Categories

(Toolkit :: Video/Audio Controls, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla10

People

(Reporter: joe, Assigned: padenot)

References

(Blocks 1 open bug)

Details

(Keywords: verified-beta, Whiteboard: [fixed-in-fx-team] [qa!])

Attachments

(2 files, 1 obsolete file)

It doesn't make a lot of sense to have a "View video" option when you've navigated to the ogg file directly, yet the menu option remains there and enabled. (Worse, it's right below the "Full screen" menuitem, which means sometimes we misclick and have to redownload the whole video again!)
FWIW, we remove "View Image" in the corresponding menu for standalone images.
Depends on: 462892
Assignee: nobody → paul
Green try push, with the new tests, fwiw : http://tbpl.mozilla.org/?tree=Try&rev=a13a40379407
Comment on attachment 552358 [details] [diff] [review]
Patch v0 - Don't show view video on standalone video.

There are a few other (unrelated) places in the front end that check for ImageDocument, might be worth a followup to consider moving then to check .mozSyntheticDocument instead... Although it's not immediately obvious to me if they should.

But anyway r+ on this for sure. :)
Attachment #552358 - Flags: review?(dolske) → review+
I forgot to add the "checkin-needed" keyword, I guess.
Keywords: checkin-needed
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
I know this had been sent to try in comment 3, but the patch no longer applied cleanly to inbound tip (I've fixed locally) & the TBPL run was over a month ago so the results don't show on TBPL any more. 

I'm therefore going to send this to try again before pushing to inbound, just to be safe (I was going to have to send a batch of checkin-neededs to try anyway, so will just include with those). 

Presuming green, will push to inbound after :-)
Keywords: checkin-needed
Sorry, I backed this out of inbound because it appears to cause intermittent failures and timeouts in test_contextmenu.html on Linux/Linux64 opt builds:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae5adccb44d4

Test failures:
https://tbpl.mozilla.org/php/getParsedLog.php?id=6504501&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=6503124&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=6503048&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=6503050&tree=Mozilla-Inbound

The failures showed up only 4 times out of 11 linux opt builds so far, so it's not surprising that this sneaked through Try.
Target Milestone: mozilla9 → ---
I noticed that the two new iframes that are used in the context menu test are referenced as global variables (missing var). This might fix the random oranges.

Pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=eb0a4927c0fc
The previous push to try had some test failures which should be fixed now:
https://tbpl.mozilla.org/?tree=Try&rev=52d7704a0c34
https://hg.mozilla.org/mozilla-central/rev/64940da7a415
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Flags: in-testsuite+
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0) Gecko/20100101 Firefox/10.0
Mozilla/5.0 (Windows NT 5.1; rv:10.0) Gecko/20100101 Firefox/10.0
Mozilla/5.0 (X11; Linux i686; rv:10.0) Gecko/20100101 Firefox/10.0

Verified the fix on the above environments: "View Video" context menu item is no longer displayed when navigating directly to video (e.g. http://videos-cdn.mozilla.net/brand/Mozilla_Firefox_Manifesto_v0.2_640.webm or file:///home/user1/Desktop/Mozilla_Firefox_Manifesto_v0.2_640.webm)
Status: RESOLVED → VERIFIED
Keywords: verified-beta
Whiteboard: [fixed-in-fx-team] → [fixed-in-fx-team] [qa!]
You need to log in before you can comment on or make changes to this bug.