Summary: | Multiple context menu actions broken for YouTube videos | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> | ||||
Component: | Media | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bugs-noreply, cdumez, cgarcia, darin, eric.carlson, ggaren, jer.noble, jonlee, mcatanzaro, pnormand, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | PC | ||||||
OS: | Linux | ||||||
Attachments: |
|
Description
Michael Catanzaro
2019-07-22 07:42:53 PDT
Broken how? How is this related with GStreamer? MediaPlayerPrivateGStreamer::canSaveMediaData() returns false on Youtube/MSE videos. WPE doesn't support context menus I'm getting a custom context menu from youtube when right clicking a video in youtube. (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. Those actions are disabled in ff and chromium context menus. Safari shows options to copy video url and open video in new window, but both options are broken too. Created attachment 374884 [details]
Patch
Michael, we can fix ephy without having to depend on this patach, since download option was already excluded.
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?
This is not disabled because it's a blob uri, but because it's a live stream. MediaPlayer::canSaveMediaData() returns False. (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? 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?
You could make canSaveMediaData() return false for "mediasourceblob" URIs and true for "blob" URIs. 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); OK from me then, but it needs an owner. 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. Committed r247901: <https://trac.webkit.org/changeset/247901> |