Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

VTTRegion::setScroll should ignore assignment of an invalid string instead of throwing SyntaxError #17567

Merged
merged 1 commit into from
Sep 9, 2023
Merged

VTTRegion::setScroll should ignore assignment of an invalid string instead of throwing SyntaxError #17567

merged 1 commit into from
Sep 9, 2023
+4 −8

Conversation

cola119
Copy link
Member

@cola119 cola119 commented Sep 8, 2023 •
edited by webkit-early-warning-system

8de0edf

VTTRegion::setScroll should ignore assignment of an invalid string instead of throwing SyntaxError
https://bugs.webkit.org/show_bug.cgi?id=261310

Reviewed by Eric Carlson.

The spec that VTTRegion::setScroll throws a SyntaxError in dead code
has been removed at w3c/webvtt#28,
and now it should ignore assignment of an invalid string.

WebKit is failing the corresponding WPT:
https://wpt.fyi/results/webvtt/api/VTTRegion/scroll.html?label=experimental&;label=master&aligned&q=webvtt

* LayoutTests/imported/w3c/web-platform-tests/webvtt/api/VTTRegion/constructor-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/webvtt/api/VTTRegion/scroll-expected.txt:
* LayoutTests/media/track/regions-webvtt/vtt-region-constructor-expected.txt:
* Source/WebCore/html/track/VTTRegion.cpp:
(WebCore::VTTRegion::setScroll):
* Source/WebCore/html/track/VTTRegion.h:

Canonical link: https://commits.webkit.org/267828@main

6ed12fc

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash c37f0c4)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ⏳ 🧪 wpe-wk2
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt   🧪 mac-wk1   🧪 gtk-wk2
✅ 🧪 api-ios   🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ⏳ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 8, 2023
@cola119 cola119 force-pushed the eng/do-not-throw-syntax-error branch from c37f0c4 to aaecf59 Compare September 8, 2023 04:26
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash aaecf59)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@cola119 cola119 force-pushed the eng/do-not-throw-syntax-error branch from aaecf59 to 6ed12fc Compare September 8, 2023 12:58
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash 6ed12fc)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@Ahmad-S792 Ahmad-S792 added the Media Bugs related to the HTML 5 Media elements. label Sep 8, 2023
@Ahmad-S792 Ahmad-S792 requested review from eric-carlson and jernoble and removed request for rniwa and cdumez September 8, 2023 19:44
@Ahmad-S792 Ahmad-S792 removed the merging-blocked Applied to prevent a change from being merged label Sep 8, 2023
@Ahmad-S792 Ahmad-S792 added the merge-queue Applied to send a pull request to merge-queue label Sep 9, 2023
@cola119
VTTRegion::setScroll should ignore assignment of an invalid string in…
8de0edf 
…stead of throwing SyntaxError

https://bugs.webkit.org/show_bug.cgi?id=261310

Reviewed by Eric Carlson.

The spec that VTTRegion::setScroll throws a SyntaxError in dead code
has been removed at w3c/webvtt#28,
and now it should ignore assignment of an invalid string.

WebKit is failing the corresponding WPT:
https://wpt.fyi/results/webvtt/api/VTTRegion/scroll.html?label=experimental&label=master&aligned&q=webvtt

* LayoutTests/imported/w3c/web-platform-tests/webvtt/api/VTTRegion/constructor-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/webvtt/api/VTTRegion/scroll-expected.txt:
* LayoutTests/media/track/regions-webvtt/vtt-region-constructor-expected.txt:
* Source/WebCore/html/track/VTTRegion.cpp:
(WebCore::VTTRegion::setScroll):
* Source/WebCore/html/track/VTTRegion.h:

Canonical link: https://commits.webkit.org/267828@main
@webkit-commit-queue
Copy link
Collaborator

Committed 267828@main (8de0edf): https://commits.webkit.org/267828@main

Reviewed commits have been landed. Closing PR #17567 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 8de0edf into WebKit:main Sep 9, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Sep 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ahmad-S792 Ahmad-S792 Ahmad-S792 left review comments

@eric-carlson eric-carlson eric-carlson approved these changes

@jernoble jernoble Awaiting requested review from jernoble

Assignees

@cola119 cola119

Labels
Media Bugs related to the HTML 5 Media elements.
Projects
None yet
Milestone
No milestone
6 participants
@cola119 @webkit-early-warning-system @webkit-commit-queue @eric-carlson @Ahmad-S792 @webkit-ews-buildbot