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.

identity/index.html Outdated Show resolved Hide resolved
@eladalon1983 @jan-ivar
Update identity/index.html
e21b7b8 
Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>
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 <yoavweiss@chromium.org>
Reviewed-by: Fr <beaufort.francois@gmail.com>
Commit-Queue: Elad Alon <eladalon@chromium.org>
Auto-Submit: Elad Alon <eladalon@chromium.org>
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#999143}
beaufortfrancois
beaufortfrancois suggested changes May 4, 2022
View reviewed changes
identity/index.html Outdated Show resolved Hide resolved
identity/index.html Show resolved Hide resolved
@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 <yoavweiss@chromium.org>
Reviewed-by: Fr <beaufort.francois@gmail.com>
Commit-Queue: Elad Alon <eladalon@chromium.org>
Auto-Submit: Elad Alon <eladalon@chromium.org>
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#999143}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3629197
Owners-Override: Srinivas Sista <srinivassista@chromium.org>
Commit-Queue: Srinivas Sista <srinivassista@chromium.org>
Reviewed-by: Srinivas Sista <srinivassista@chromium.org>
Cr-Commit-Position: refs/branch-heads/5005@{#466}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
@copybara-github
[Capture Handle] Use a blank event
3c4f26b 
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 <yoavweiss@chromium.org>
Reviewed-by: Fr <beaufort.francois@gmail.com>
Commit-Queue: Elad Alon <eladalon@chromium.org>
Auto-Submit: Elad Alon <eladalon@chromium.org>
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#999143}
NOKEYCHECK=True
GitOrigin-RevId: 26462712dac66ee2120c02dece5ec85d76d6de44
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