Bug 260409
| Summary: | Empty files should not succeed in loading (as WebVTT) | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Ahmad Saleem <ahmad.saleem792> |
| Component: | Media | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | annevk, destra, eric.carlson, jer.noble, karlcow, kohei.ueno119, webkit-bug-importer |
| Priority: | P2 | Keywords: | BrowserCompat, InRadar, WPTImpact |
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| URL: | http://wpt.live/webvtt/parsing/file-parsing/signature-invalid.html | ||
| Bug Depends on: | 179370 | ||
| Bug Blocks: | |||
Ahmad Saleem
Hi Team,
While going through Blink's commit, I came across another potential merge:
Blink Commit: https://chromium.googlesource.com/chromium/src.git/+/0983d4b64cb5f519684f773daa27b2ac9b8f8688
WebKit Source: https://searchfox.org/wubkat/source/Source/WebCore/loader/TextTrackLoader.cpp#120
WPT Test Case: https://wpt.fyi/results/webvtt/parsing/file-parsing/signature-invalid.html?label=master&label=experimental&aligned=&q=signature
WPT Test Case Live Link: http://wpt.live/webvtt/parsing/file-parsing/signature-invalid.html
^ It is about first failure - 'signature, empty'.
Just wanted to raise, so we can fix it. Adding "WPTImpact" and "BrowserCompat" tags.
Thanks!
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Radar WebKit Bug Importer
<rdar://problem/114118292>
Ahmad Saleem
NOTE - Doing 1-1 Blink Merge leads to two WPT regressions:
html/semantics/embedded-content/media-elements/track/track-element
/track-add-remove-cue.html
and
html/semantics/embedded-content/media-elements/track/track-element/track-text-track-cue-list.html
___
PR attempt: https://github.com/WebKit/WebKit/pull/16863
____
NOTE: We should sync 'webvtt' and look into why the tests are timing out and fix it.
Still investigating but closing my PR for time being.
Anne van Kesteren
I was thinking of fixing this in TextTrackLoader::notifyFinished by changing
if (m_cueParser)
m_cueParser->fileFinished();
to
if (m_cueParser)
m_cueParser->fileFinished();
else
m_state = Failed;
but I'm not sure if that would similarly regress tests.
However, looking closely at your commit it seems that WebKit would still invoke processNewCueData for the non-Failed states whereas Chromium would not (at least not at that point in time).
Syncing webvtt WPT is probably a good next step here. I can maybe do that today between meetings.
Kueno
@Ahmad Saleem @Anne van Kesteren
Are you working on this issue? If not, I'd be happy to take it over.
Ahmad Saleem
(In reply to Kueno from comment #4)
> @Ahmad Saleem @Anne van Kesteren
>
> Are you working on this issue? If not, I'd be happy to take it over.
I am not. Go ahead and take it. You are doing amazing job!! We all really appreciate it.
Kueno
Pull request: https://github.com/WebKit/WebKit/pull/19401
EWS
Committed 269646@main (d40154a4fe8f): <https://commits.webkit.org/269646@main>
Reviewed commits have been landed. Closing PR #19401 and removing active labels.