Bug 199999 – Multiple context menu actions broken for YouTube videos

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Michael Catanzaro]

The following context menu actions are broken on youtube.com videos:

WEBKIT_CONTEXT_MENU_ACTION_OPEN_VIDEO_IN_NEW_WINDOW
WEBKIT_CONTEXT_MENU_ACTION_DOWNLOAD_VIDEO_TO_DISK

The former tries to load a blob: URI in a new tab (impossible) and the later just does nothing (probably because of MSE). I'm really not sure what to do about either of these, other than that leaving them broken isn't OK. My only idea is to change WebKitHitTestResult to report that these videos are not videos, but that's... meh.

Not tested, but also suspect:

WEBKIT_CONTEXT_MENU_ACTION_OPEN_AUDIO_IN_NEW_WINDOW
WEBKIT_CONTEXT_MENU_ACTION_DOWNLOAD_AUDIO_TO_DISK

[Philippe Normand]

Broken how?
How is this related with GStreamer?

[Philippe Normand]

MediaPlayerPrivateGStreamer::canSaveMediaData() returns false on Youtube/MSE videos.

[Carlos Garcia Campos]

WPE doesn't support context menus

[Carlos Garcia Campos]

I'm getting a custom context menu from youtube when right clicking a video in youtube.

[Michael Catanzaro]

(In reply to Carlos Garcia Campos from comment #4)
> I'm getting a custom context menu from youtube when right clicking a video
> in youtube.

First right-click brings up the website's custom context menu. Then right-click a second time to get Epiphany's.

[Carlos Garcia Campos]

Those actions are disabled in ff and chromium context menus.

[Carlos Garcia Campos]

Safari shows options to copy video url and open video in new window, but both options are broken too.

[Carlos Garcia Campos]

Created attachment 374884 [details]
Patch

Michael, we can fix ephy without having to depend on this patach, since download option was already excluded.

[Michael Catanzaro]

Comment on attachment 374884 [details]
Patch

Nice, but I wonder if it's semantically right. E.g. a video with a blob: URI should actually be downloadable if it's not using MSE, but copy media link would result in a meaningless URI and open in new window wouldn't work. Right?

[Carlos Garcia Campos]

This is not disabled because it's a blob uri, but because it's a live stream. MediaPlayer::canSaveMediaData() returns False.

[Michael Catanzaro]

(In reply to Carlos Garcia Campos from comment #10)
> This is not disabled because it's a blob uri, but because it's a live
> stream. MediaPlayer::canSaveMediaData() returns False.

OK, but that just proves my point: this will fail if you have a normal video as a blob URI, right?

[Michael Catanzaro]

Comment on attachment 374884 [details]
Patch

I guess this is necessary regardless, though, because if the media is not downloadable then surely it doesn't have a usable link... is that always true?

[Philippe Normand]

You could make canSaveMediaData() return false for "mediasourceblob" URIs and true for "blob" URIs.

[Carlos Garcia Campos]

If the media is not downloadable the url is useless I think, and it can't be opened in a new tab/window either. This is what ff and chromium does. In the case of chromium, it allows to copy the link url, but I'm not sure if that's on purpose or just a bug. Firefox has all the options disabled. In chromium the have this:

    case IDC_CONTENT_CONTEXT_OPENAVNEWTAB:
      // Currently, a media element can be opened in a new tab iff it can                                                                                                                     
      // be saved. So rather than duplicating the MediaCanSave flag, we rely                                                                                                                  
      // on that here.                                                                                                                                                                        
      return !!(params_.media_flags & WebContextMenuData::kMediaCanSave);

[Michael Catanzaro]

OK from me then, but it needs an owner.

[Carlos Garcia Campos]

This doesn't require an owner because it's a change in WebCore, not WebKit2. But still, I want to make sure Apple is also ok with this change before landing.

[Radar WebKit Bug Importer]

<rdar://problem/53585686>

[Carlos Garcia Campos]

Committed r247901: <https://trac.webkit.org/changeset/247901>