View Video not present on video context menu

RESOLVED FIXED in seamonkey2.0b2

Status

RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: BijuMailList, Assigned: InvisibleSmiley)

Tracking

Trunk
seamonkey2.0b2

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

10 years ago
"View Video" item not present on video context menu 

Step:-
1. goto http://www.double.co.nz/video_test/test5.html
2. wait video to load the initial screen
3. bring mouse over the video element
4. do right click to get context menu.

result:-
user dont see "View Video" item

result:-
user see "View Video" item
above "Copy Video Location" item

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) 
Gecko/20090316 SeaMonkey/2.0b1pre
(Assignee)

Comment 1

9 years ago
Created attachment 396133 [details] [diff] [review]
proposed patch

Ports bug 462294 and the corresponding part of bug 496832.
Assignee: nobody → jh
Status: NEW → ASSIGNED
Attachment #396133 - Flags: superreview?(neil)
Attachment #396133 - Flags: review?(neil)

Comment 2

9 years ago
Comment on attachment 396133 [details] [diff] [review]
proposed patch

Is it possible to check that the video location is not the same as the document location?

Also, the access key should be a lower case "i" to match "View".
(Assignee)

Comment 3

9 years ago
Created attachment 396200 [details] [diff] [review]
patch v1a

(In reply to comment #2)
> (From update of attachment 396133 [details] [diff] [review])
> Is it possible to check that the video location is not the same as the document
> location?

See bug 462294 comment 5: that's bug 462892. Short version: no solution available yet. Once there is we can port it.
Attachment #396133 - Attachment is obsolete: true
Attachment #396200 - Flags: superreview?(neil)
Attachment #396200 - Flags: review?(neil)
Attachment #396133 - Flags: superreview?(neil)
Attachment #396133 - Flags: review?(neil)

Comment 4

9 years ago
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 396133 [details] [diff] [review] [details])
> > Is it possible to check that the video location is not the same as the document location?
> Short version: no solution available yet. Once there is we can port it.
But that will never happen for 1.9.1, which is why I suggested comparing the location, since only a full-screen video will have the same location.
(Assignee)

Comment 5

9 years ago
Created attachment 396234 [details] [diff] [review]
patch v2

This is as close as I could get.
Attachment #396200 - Attachment is obsolete: true
Attachment #396234 - Flags: superreview?(neil)
Attachment #396234 - Flags: review?(neil)
Attachment #396200 - Flags: superreview?(neil)
Attachment #396200 - Flags: review?(neil)

Comment 6

9 years ago
Comment on attachment 396234 [details] [diff] [review]
patch v2

>+        this.showItem( "context-viewvideo", this.onVideo);
>+        this.setItemAttr( "context-viewvideo", "disabled", !this.mediaURL ||
>+                         this.mediaURL == this.target.ownerDocument.location.href);
>+
Nit: hide the menuitem rather than disabling it, like view image does
Nit: compare the URL against content.location.href, in case it's in a frame (or use this.inFrame)
Nit: indentation was odd, but then it's odd for view image...
Attachment #396234 - Flags: superreview?(neil)
Attachment #396234 - Flags: superreview+
Attachment #396234 - Flags: review?(neil)
Attachment #396234 - Flags: review+
(Assignee)

Comment 7

9 years ago
Created attachment 396525 [details] [diff] [review]
patch v3
[Checkin: Comment 8]
Attachment #396234 - Attachment is obsolete: true
Attachment #396525 - Flags: superreview+
Attachment #396525 - Flags: review+
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Comment on attachment 396525 [details] [diff] [review]
patch v3
[Checkin: Comment 8]


http://hg.mozilla.org/comm-central/rev/a60ce0d7786a
Attachment #396525 - Attachment description: patch v3 → patch v3 [Checkin: Comment 8]
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0b2

Updated

6 years ago
Duplicate of this bug: 462714
You need to log in before you can comment on or make changes to this bug.