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

Drop captureHandle() from the event #57

Merged
merged 4 commits into from
May 6, 2022
Merged

Drop captureHandle() from the event #57

merged 4 commits into from
May 6, 2022

Conversation

eladalon1983
Copy link
Member

@eladalon1983 eladalon1983 commented May 3, 2022 •
edited
Loading

Fixes #50.

@eladalon1983 eladalon1983 requested a review from jan-ivar May 3, 2022 20:39
jan-ivar
jan-ivar requested changes May 3, 2022
View reviewed changes
identity/index.html Outdated
Comment on lines 369 to 372
[Exposed=Window]
interface CaptureHandleChangeEvent : Event {
constructor(optional CaptureHandleChangeEventInit init = {});
[SameObject] CaptureHandle captureHandle();
constructor();
};
Copy link
Member

Choose a reason for hiding this comment

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

A new interface is only necessary to add custom members, so we should remove it entirely, or it is JS-observable. E.g. WebRTC has several events that are plain.

@eladalon1983 @jan-ivar
Update identity/index.html
e21b7b8 
Co-authored-by: Jan-Ivar Bruaroey <[email protected]>
aarongable pushed a commit to chromium/chromium that referenced this pull request May 3, 2022
[Capture Handle] Use a blank event
2646271 
Because the event is fired synchronously, rather than from a queued
task, there is no need to make a distinction between the value of
track.getCaptureHandle() and the value at the time the event was fired.
Therefore, this CL removes the momentary value of the Capture Handle at
the time the event was fired. Applications can instead read that value
through calls to `event.target.getCaptureHandle()`.

PR: w3c/mediacapture-handle#57

Bug: 1322174
Change-Id: Iffe4bdafb3900fe4f1ccac83aaee5e5a06f9ffaf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3625272
Commit-Queue: Yoav Weiss <[email protected]>
Reviewed-by: Fr <[email protected]>
Commit-Queue: Elad Alon <[email protected]>
Auto-Submit: Elad Alon <[email protected]>
Reviewed-by: Yoav Weiss <[email protected]>
Cr-Commit-Position: refs/heads/main@{#999143}
beaufortfrancois
beaufortfrancois suggested changes May 4, 2022
View reviewed changes
@jan-ivar
Copy link
Member

jan-ivar commented May 5, 2022

Editor's can integrate once all CaptureHandleChangeEvent WebIDL is removed.

@eladalon1983 eladalon1983 merged commit ee0cfbc into w3c:main May 6, 2022
@beaufortfrancois beaufortfrancois mentioned this pull request May 9, 2022
Open
darinwf pushed a commit to neevaco/chromium that referenced this pull request Jun 2, 2022
[Cherry-Pick][Capture Handle] Use a blank event
b1a357c 
Because the event is fired synchronously, rather than from a queued
task, there is no need to make a distinction between the value of
track.getCaptureHandle() and the value at the time the event was fired.
Therefore, this CL removes the momentary value of the Capture Handle at
the time the event was fired. Applications can instead read that value
through calls to `event.target.getCaptureHandle()`.

PR: w3c/mediacapture-handle#57

(cherry picked from commit 2646271)

Bug: 1322174
Change-Id: Iffe4bdafb3900fe4f1ccac83aaee5e5a06f9ffaf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3625272
Commit-Queue: Yoav Weiss <[email protected]>
Reviewed-by: Fr <[email protected]>
Commit-Queue: Elad Alon <[email protected]>
Auto-Submit: Elad Alon <[email protected]>
Reviewed-by: Yoav Weiss <[email protected]>
Cr-Original-Commit-Position: refs/heads/main@{#999143}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3629197
Owners-Override: Srinivas Sista <[email protected]>
Commit-Queue: Srinivas Sista <[email protected]>
Reviewed-by: Srinivas Sista <[email protected]>
Cr-Commit-Position: refs/branch-heads/5005@{#466}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jan-ivar jan-ivar jan-ivar requested changes

@beaufortfrancois beaufortfrancois beaufortfrancois requested changes

Assignees
No one assigned
Labels
Editors can integrate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CaptureHandleChangeEvent should just be Event
3 participants
@eladalon1983 @jan-ivar @beaufortfrancois