Summary: | REGRESSION(243836@main): [GStreamer] Broke videos in yelp | ||
---|---|---|---|
Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> |
Component: | Media | Assignee: | Michael Catanzaro <mcatanzaro> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | bugs-noreply, jer.noble, mcatanzaro, philn, pnormand, tpopela, webkit-bug-importer |
Priority: | P2 | Keywords: | InRadar |
Version: | WebKit Nightly Build | ||
Hardware: | PC | ||
OS: | Linux | ||
See Also: | https://bugzilla.redhat.com/show_bug.cgi?id=2099334 |
Description
Michael Catanzaro
2022-06-29 19:09:15 PDT
BTW this hack was needed because I failed to find any documentation with videos remaining so that I could give an easy reproducer. We only noticed because RHEL 8 still has some videos. Anyway, the change that broke things is: diff --git a/Source/WebCore/html/HTMLMediaElement.cpp b/Source/WebCore/html/HTMLMediaElement.cpp index ff346f25f75e..345a7e310251 100644 --- a/Source/WebCore/html/HTMLMediaElement.cpp +++ b/Source/WebCore/html/HTMLMediaElement.cpp @@ -1472,10 +1472,12 @@ void HTMLMediaElement::loadResource(const URL& initialURL, ContentType& contentT } URL url = initialURL; - if (!url.isEmpty() && !frame->loader().willLoadMediaElementURL(url, *this)) { +#if PLATFORM(COCOA) + if (url.isLocalFile() && !frame->loader().willLoadMediaElementURL(url, *this)) { mediaLoadingFailed(MediaPlayer::NetworkState::FormatError); return; } +#endif #if ENABLE(CONTENT_EXTENSIONS) if (RefPtr documentLoader = frame->loader().documentLoader()) { Jer's changelog contains reasoning for making this Cocoa-specific: """ Drive-by fix: to allow this change to be testable, we must revert a change which calls all the network delegate callbacks with empty data. This was necessary at the time because (at least for Cocoa ports) media loading happened outside WebCore's loader path. Currently, all http(s), data, blob, and custom protocol schemes are loaded through WebCore, leaving file:// URLs as the remaining protocol type that needs custom handling, and only on Cocoa ports. """ Notice the GStreamer log contains errors related to the URI scheme: 0:00:00.146452332 4822 0x561c5fcb4600 ERROR webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1714:handleMessage: Error 12: No URI handler implemented for "bogus-help". (url=bogus-help:/gnome-help/figures/gnome-windows-and-workspaces.webm) yelp seems to be using this bogus-help URI scheme to trick WebKit into... something something, I don't understand. It was added here: https://gitlab.gnome.org/GNOME/yelp/-/commit/04683e89cef14f56e4c3d1595424e109f12f5d06 This fixes yelp: diff --git a/Source/WebCore/html/HTMLMediaElement.cpp b/Source/WebCore/html/HTMLMediaElement.cpp index 97f855b6b592..95c10e228b81 100644 --- a/Source/WebCore/html/HTMLMediaElement.cpp +++ b/Source/WebCore/html/HTMLMediaElement.cpp @@ -1503,12 +1503,10 @@ void HTMLMediaElement::loadResource(const URL& initialURL, ContentType& contentT } URL url = initialURL; -#if PLATFORM(COCOA) - if (url.isLocalFile() && !frame->loader().willLoadMediaElementURL(url, *this)) { + if (!url.isEmpty() && !frame->loader().willLoadMediaElementURL(url, *this)) { mediaLoadingFailed(MediaPlayer::NetworkState::FormatError); return; } -#endif #if ENABLE(CONTENT_EXTENSIONS) if (RefPtr documentLoader = frame->loader().documentLoader()) { Phil, Jer, do you want a pull request for this? """ Drive-by fix: to allow this change to be testable, we must revert a change which calls all the network delegate callbacks with empty data. This was necessary at the time because (at least for Cocoa ports) media loading happened outside WebCore's loader path. Currently, all http(s), data, blob, and custom protocol schemes are loaded through WebCore, leaving file:// URLs as the remaining protocol type that needs custom handling, and only on Cocoa ports. """ Well, IIUC, GStreamer ports also apply here, file loading doesn't go through WebCore, but with a pure-GStreamer filesrc element. Can you try this test? if ((!url.isEmpty() || url.isLocalFile()) && !frame->loader().willLoadMediaElementURL(url, *this)) { (In reply to Philippe Normand from comment #3) > if ((!url.isEmpty() || url.isLocalFile()) && > !frame->loader().willLoadMediaElementURL(url, *this)) { That works. well actually an empty url can't be local I suppose, so my suggestion doesn't make much sense. Created a yelp bug: https://gitlab.gnome.org/GNOME/yelp/-/issues/193 Pull request: https://github.com/WebKit/WebKit/pull/2635 Committed 252746@main (6cbee1cfa19b): <https://commits.webkit.org/252746@main> Reviewed commits have been landed. Closing PR #2635 and removing active labels. |