483727 - View Video not present on video context menu

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Biju]

"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

Comment 1

[Jens Hatlak (:InvisibleSmiley)]

Attachment: proposed patch

Ports bug 462294 and the corresponding part of bug 496832.

Comment 2

[[e-mail]]

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".

Comment 3

[Jens Hatlak (:InvisibleSmiley)]

Attachment: 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.

Comment 4

[[e-mail]]

(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.

Comment 5

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v2

This is as close as I could get.

Comment 6

[[e-mail]]

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...

Comment 7

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v3 [Checkin: Comment 8]

Comment 8

[Serge Gautherie (:sgautherie)]

Comment on attachment 396525 [details] [diff] [review]
patch v3
[Checkin: Comment 8]


http://hg.mozilla.org/comm-central/rev/a60ce0d7786a