ImageCapture applyConstraints() should reject if no constraints are recognised |
||||||||||
Issue description
applyConstraints() should reject if all constraints are not recognised, i.e.,
(assuming |zoom| is supported)
theTrack.applyConstraints({ advanced: [{zoom : 200}]});
should resolve, whereas:
theTrack.applyConstraints({ advanced: [{blergh : 100}]});
should reject.
(Other notes: tested using
https://rawgit.com/yellowdoge/demos/master/imagecapture.html
and
theTrack.applyConstraints({ advanced: [{zoom : 200}]}).then(()=>{console.log('yay');}).catch((e)=>{console.error('boo: ' + e.message);});
theTrack.applyConstraints({ advanced: [{blergh : 100}]}).then(()=>{console.log('yay');}).catch((e)=>{console.error('boo: ' + e.message);});
)
,
May 17 2017
Note that the protection cannot be total "because WebIDL drops any unsupported names from the dictionary holding the constraints, so the browser does not see them and the unsupported names end up being silently ignored." (see "Note" in https://w3c.github.io/mediacapture-main/#constrainable-interface)
,
May 17 2017
,
May 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/268fc24e770401f31288f09d8de7f17d86dc0220 commit 268fc24e770401f31288f09d8de7f17d86dc0220 Author: Miguel Casas-Sanchez <mcasas@chromium.org> Date: Thu May 18 06:10:17 2017 Image Capture: reject applyConstraints() if there isn't any recognized member This CL corrects the code so that theTrack.applyConstraints({ advanced: [{blergh : 100}]}); will be rejected due to unknown dictionary members, also adding a Layout Tests entry. To reject, the logic in HasNonImageCaptureConstraints() is inverted, checking for the presence of image constraints (instead of checking for the absence of non-image constraints, which lets an empty dictionary through) -- Note that WebIdl will silently drop unsupported dictionary members. Bug: 723779 Change-Id: I3c911ea4d319deab828c135a9f88a3d5af0b06bf Reviewed-on: https://chromium-review.googlesource.com/508171 Commit-Queue: Miguel Casas <mcasas@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/master@{#472699} [modify] https://crrev.com/268fc24e770401f31288f09d8de7f17d86dc0220/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints-reject.html [modify] https://crrev.com/268fc24e770401f31288f09d8de7f17d86dc0220/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
,
May 18 2017
,
May 18 2017
Can you please mark which OS this affects?
,
May 18 2017
,
May 18 2017
,
May 18 2017
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 18 2017
,
May 18 2017
,
May 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/372a89d7cf9bc06c0c00fd1394e6a727ef5619b4 commit 372a89d7cf9bc06c0c00fd1394e6a727ef5619b4 Author: Miguel Casas-Sanchez <mcasas@chromium.org> Date: Thu May 18 20:34:59 2017 Image Capture: reject applyConstraints() if there isn't any recognized member This CL corrects the code so that theTrack.applyConstraints({ advanced: [{blergh : 100}]}); will be rejected due to unknown dictionary members, also adding a Layout Tests entry. To reject, the logic in HasNonImageCaptureConstraints() is inverted, checking for the presence of image constraints (instead of checking for the absence of non-image constraints, which lets an empty dictionary through) -- Note that WebIdl will silently drop unsupported dictionary members. Bug: 723779 Change-Id: I3c911ea4d319deab828c135a9f88a3d5af0b06bf Reviewed-on: https://chromium-review.googlesource.com/508171 Commit-Queue: Miguel Casas <mcasas@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#472699} Review-Url: https://codereview.chromium.org/2887913005 . Cr-Commit-Position: refs/branch-heads/3071@{#621} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [modify] https://crrev.com/372a89d7cf9bc06c0c00fd1394e6a727ef5619b4/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints-reject.html [modify] https://crrev.com/372a89d7cf9bc06c0c00fd1394e6a727ef5619b4/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
,
May 20 2017
,
May 20 2017
According to spec, theTrack.applyConstraints({ advanced: [{blergh : 100}]}) should not reject.
The unsupported constraint should be ignored and it should produce the same result as passing no constraints, which should not reject.
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 Deleted