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.

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

Re-add <source media> for media elements #9341

Merged
merged 4 commits into from
Jun 12, 2023
Merged

Conversation

zcorpan
Copy link
Member

@zcorpan zcorpan commented May 26, 2023

Fixes #6363

(See WHATWG Working Mode: Changes for more details.)


/embedded-content.html ( diff )
/indices.html ( diff )
/media.html ( diff )

@foolip
Copy link
Member

foolip commented May 26, 2023

@dalecurtis what do you think about implementing this in Chromium?

This was once implemented in Chromium (inherited from WebKit) but I removed it in https://bugs.chromium.org/p/chromium/issues/detail?id=338197. In hindsight I think that was the wrong call, and I'm glad it's being reverted in the spec now.

source Show resolved Hide resolved
@dalecurtis
Copy link
Contributor

It seems reasonable and is something folks have been asking for. I'm not sure about implementation complexity, but the removed code doesn't look too large -- so it shouldn't be too bad.

I can only think of one minor issue at this time: It may interact in ways surprising to developers if an error occurs during loading. I.e., it could look like a resource wasn't selected due to the query but instead due to a load error that causes the next source child to be loaded.

@zcorpan
Copy link
Member Author

zcorpan commented May 29, 2023

@dalecurtis the same applies for type. We don't currently have a way to indicate why a resource wasn't loaded when using source elements (when using <video src> reason is available in video.error). It would be possible to expose with the error event (in the "Failed with elements" step), but this would be a separate issue.

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, seems like most of the editorial part of the work was already done due to picture.

source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
@annevk annevk added the needs tests Moving the issue forward requires someone to write tests label May 31, 2023
@zcorpan
Copy link
Member Author

zcorpan commented May 31, 2023

@annevk thanks, nits fixed.

zcorpan added a commit to web-platform-tests/wpt that referenced this pull request May 31, 2023
@zcorpan zcorpan removed the needs tests Moving the issue forward requires someone to write tests label May 31, 2023
@zcorpan
Copy link
Member Author

zcorpan commented May 31, 2023

I've written tests and filed bugs.

zcorpan added a commit to web-platform-tests/wpt that referenced this pull request Jun 12, 2023
@zcorpan zcorpan merged commit 9e9a673 into main Jun 12, 2023
@zcorpan zcorpan deleted the zcorpan/re-add-source-media branch June 12, 2023 12:20
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jun 22, 2023
…ts, a=testonly

Automatic update from web-platform-tests
HTML: Add <source media> to media elements

See whatwg/html#9341

--

wpt-commits: 0f4b390234ab06f8841c9e89c74afc04c66c7f91
wpt-pr: 40330
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this pull request Jun 22, 2023
…ts, a=testonly

Automatic update from web-platform-tests
HTML: Add <source media> to media elements

See whatwg/html#9341

--

wpt-commits: 0f4b390234ab06f8841c9e89c74afc04c66c7f91
wpt-pr: 40330
@janwidmer
Copy link

janwidmer commented Dec 7, 2023

@scottjehl any idea, when this feature lands in Firefox / Chrome Production?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

HTML Video Element: Proposal For Reintroduction of Media in Source Elements
6 participants