Closed Bug 556563 Opened 10 years ago Closed 8 years ago
Disable/remove "View video" when you're already viewing the video
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.
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+
Any reason why this wasn't landed?
I forgot to add the "checkin-needed" keyword, I guess.
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 :-)
Target Milestone: --- → mozilla9
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.
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
Fixed the tests.
Attachment #571117 - Attachment is obsolete: true
Pushed to fx-team: https://hg.mozilla.org/integration/fx-team/rev/64940da7a415
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
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
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.