CL1: Enable MediaCapture feature on desktop (experimental) [chromium/src : main]
Helmut Januschka (Gerrit)
Helmut Januschka added 1 comment![]( "Open in Gerrit")
hello reviewers, this is CL1 of 3 (+ one more for final enablement)
design doc is here: https://docs.google.com/document/d/1p2QlRMDbHUc-GneEUuf-yHXRCqWqJijZSFD_KErqdnQ/edit?tab=t.0
also please see: https://chromium-review.googlesource.com/c/chromium/src/+/7594822/comments/304c6eca_e8c8ae30
Related details
- David Bokan
- Manuel Rego
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Bokan (Gerrit)
David Bokan voted and added 3 comments![]( "Open in Gerrit")
Votes added by David Bokan
| Code-Review | +1 |
3 comments
Btw, have you followed the new feature process? (realizing this is extending an existing feature to desktop but parts of this are probably still relevant). In particular, you might want to send an intent to prototype to blink-dev if you haven't yet as well as a chromestatus entry.
lgtm otherwise, w/questions about test failures.
[FAIL] capture on HTMLInputElement must enqueue an attributeChanged reaction when adding new attributeAre these failures to be fixed in a followup CL?
FAIL input.capture should be false (of type boolean). Was (of type string).Interesting - why the type difference?
Related details
- Helmut Januschka
- Manuel Rego
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka added 4 comments![]( "Open in Gerrit")
Btw, have you followed the new feature process? (realizing this is extending an existing feature to desktop but parts of this are probably still relevant). In particular, you might want to send an intent to prototype to blink-dev if you haven't yet as well as a chromestatus entry.
lgtm otherwise, w/questions about test failures.
Btw, have you followed the new feature process?
sorry i am an external, where could i read/catch up on the process?
here is the chromestatus: https://chromestatus.com/feature/5154771310870528
and the blink-dev i2p: https://groups.google.com/a/chromium.org/g/blink-dev/c/Z8MK8Q_7DQI/m/7u5tD0JDAQAJ
anything else i'd need todo?
[FAIL] capture on HTMLInputElement must enqueue an attributeChanged reaction when adding new attributeAre these failures to be fixed in a followup CL?
the `capture` IDL attribute is missing `[CEReactions]` in html_input_element.idl. This was never caught because desktop didn't expose `capture` and Android doesn't run blink web_tests. guess the fix should be a follow up?
FAIL input.capture should be false (of type boolean). Was (of type string).Interesting - why the type difference?
The `capture` attribute changed from `boolean` n the spec (values: `""`, `"user"`, `"environment"`). Blink's IDL correctly implements the current spec, but this test was written for the old boolean spec. Before this CL, desktop returned `undefined`; now it returns strings per spec.
Related details
- David Bokan
- Manuel Rego
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Bokan (Gerrit)
David Bokan voted and added 4 comments![]( "Open in Gerrit")
Votes added by David Bokan
| Code-Review | +1 |
Helmut JanuschkaBtw, have you followed the new feature process? (realizing this is extending an existing feature to desktop but parts of this are probably still relevant). In particular, you might want to send an intent to prototype to blink-dev if you haven't yet as well as a chromestatus entry.
lgtm otherwise, w/questions about test failures.
Btw, have you followed the new feature process?
sorry i am an external, where could i read/catch up on the process?
here is the chromestatus: https://chromestatus.com/feature/5154771310870528
and the blink-dev i2p: https://groups.google.com/a/chromium.org/g/blink-dev/c/Z8MK8Q_7DQI/m/7u5tD0JDAQAJanything else i'd need todo?
sorry i am an external, where could i read/catch up on the process?
Oops, sorry, I thought I linked it but forgot: https://www.chromium.org/blink/launching-features/
Everything here is public.
anything else i'd need todo?
I think you're good, thanks. (nit: would you mind linking to the relevant spec in the commit message and in the chromestatus entry? I think this is it? https://www.w3.org/TR/html-media-capture)
{"enable-desktop-media-capture",wondering if you need this separately in chrome://flags? We already have chrome://flags/#enable-experimental-web-platform-features which turns on all experimental Blink features in runtime_enabled_features.json5 so if this is just to let devs/users try it out I think they can use that flag?
[FAIL] capture on HTMLInputElement must enqueue an attributeChanged reaction when adding new attributeHelmut JanuschkaAre these failures to be fixed in a followup CL?
the `capture` IDL attribute is missing `[CEReactions]` in html_input_element.idl. This was never caught because desktop didn't expose `capture` and Android doesn't run blink web_tests. guess the fix should be a follow up?
- Helmut Januschka
- Manuel Rego
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka added 1 comment![]( "Open in Gerrit")
{"enable-desktop-media-capture",wondering if you need this separately in chrome://flags? We already have chrome://flags/#enable-experimental-web-platform-features which turns on all experimental Blink features in runtime_enabled_features.json5 so if this is just to let devs/users try it out I think they can use that flag?
but could it then be killswitched? like lets say, i broke something, how could that be selectivly stopped?
Related details
- Manuel Rego
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |