Blink IDL compatibility: fix nullable <=> non-nullable attributes / arguments |
||||||||||||||||
Issue description$ git grep -E '(FIXME|TODO).*null' -- '*.idl' This finds some cases that have already been annotated as not matching the spec. For each of these: 1. Check that the spec hasn't changed, i.e. that it's still non-nullable. 2. Write a test for the behavior when null is used. (Not always observable.) 3. Run the tests on Edge, Firefox and Safari and see if any of them match the spec. 4. If in doubt about Firefox, check https://github.com/mozilla/gecko-dev/tree/master/dom/webidl If at least one other engine already has the per-spec behavior, change Blink to match. If it looks like nullable actually be the better behavior, instead file a spec issue for discussion. At the time of filing this bug, there are the cases that seems most actionable: DataTransferItemList#add() file argument Document#documentURI attribute (readonly, could be unobservable) Blob#slice() contentType argument History#pushState() and replaceState() title arguments There are also a number of event init dicts, but these are lower priority because event constructors aren't used much, at least not compared to something like blob.slice(). ⛆ |
|
|
,
Nov 7 2016
,
Nov 15 2016
,
Nov 15 2016
Below is the list I generated. I am going to verify each of them, and see if a bug has been created, then try to fix a few: DataTransferItemList#add() file argument Document#documentURI attribute Document# typeExtension arguments IntersectionObserverEntry#rootBounds attribute Blob#slice() contentType argument History#pushState() title argument ShareWorker#constructor() name argument RTCPeerConnection#createDataChannel() label argument RTCSessionDescription#type attribute, sdp attribute GamepadEventInit#gamepad VRDisplayEventInit#display MIDIConnectionEventInit#port MIDIMessageEventInit#data
,
Nov 15 2016
Yay! Blob#slice() I suspect is easy to fix and that some other browser already does the right thing, so might be a good place to start.
,
Nov 17 2016
About to submit fix to Blob#slice() Working on DataTransferItemList#add() now
,
Nov 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/68dddcc6a3b8aea8c1836ad353e81993ef27d1ee commit 68dddcc6a3b8aea8c1836ad353e81993ef27d1ee Author: lunalu <lunalu@chromium.org> Date: Tue Nov 22 17:09:42 2016 Make Blob#slice() contentType argument to be non nullable. Compatibility: new Blob().slice(0,0,null).type Edge: "null" (match with the spec) Firefox: "null" (match with the spec) Chrome: "" Safari: "" BUG=662005 Review-Url: https://codereview.chromium.org/2509833002 Cr-Commit-Position: refs/heads/master@{#433890} [add] https://crrev.com/68dddcc6a3b8aea8c1836ad353e81993ef27d1ee/third_party/WebKit/LayoutTests/fast/files/blob-slice-type.html [modify] https://crrev.com/68dddcc6a3b8aea8c1836ad353e81993ef27d1ee/third_party/WebKit/LayoutTests/http/tests/local/fileapi/script-tests/send-sliced-dragged-file.js [modify] https://crrev.com/68dddcc6a3b8aea8c1836ad353e81993ef27d1ee/third_party/WebKit/Source/core/fileapi/Blob.idl
,
Nov 22 2016
A word of caution about "RTCPeerConnection#createDataChannel() label argument". I was changing the dictionary handling to match spec and tried to make label a non-nullable USVString with [TreatNullAs=EmptyString], but it turns out that [TreatNullAs=EmptyString] doesn't actually work with USVString. I haven't filed an issue yet, but it's probably not worth changing the label argument just a little.
,
Nov 22 2016
,
Nov 22 2016
Thanks for the heads up. Also working on RTCSessionDescription#type and sdp now
,
Nov 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/11a9aad4ec1d0ae6099a3721813f9a20cfc6fe6b commit 11a9aad4ec1d0ae6099a3721813f9a20cfc6fe6b Author: lunalu <lunalu@chromium.org> Date: Wed Nov 23 09:14:24 2016 DataTransferItemList#add() file argument should not be nullable Compatibility: dataTransfer.items.add(null) Edge: does not throw Firefox 49: dataTransfer.items is undefined Firefox 50: throw as expected Safari: dataTransfer.items is undefined No use counter is added as we will likely get informed if any problems come up https://bugzilla.mozilla.org/show_bug.cgi?id=906420 BUG=662005 Review-Url: https://codereview.chromium.org/2508773005 Cr-Commit-Position: refs/heads/master@{#434137} [modify] https://crrev.com/11a9aad4ec1d0ae6099a3721813f9a20cfc6fe6b/third_party/WebKit/LayoutTests/fast/events/drag-dataTransferItemList-expected.txt [modify] https://crrev.com/11a9aad4ec1d0ae6099a3721813f9a20cfc6fe6b/third_party/WebKit/LayoutTests/fast/events/drag-dataTransferItemList.html [modify] https://crrev.com/11a9aad4ec1d0ae6099a3721813f9a20cfc6fe6b/third_party/WebKit/Source/core/clipboard/DataTransferItemList.idl
,
Nov 23 2016
For RTCSessionDescription, I would couple that change with issue 662898, making type required in the init dict, as that's what will be required to return something non-null for type. The [Measure] bits in RTCSessionDescription.idl were added quite recently, so I'd say it's best to just wait with that effort for a while until we have some data from them. Of the things mentioned in #4, I'd guess that Document#documentURI is the most likely to be a real-world interop issue, and the event init ones are probably the easiest and least risky to fix.
,
Nov 23 2016
Thanks for the information. Working on Document#documentURI attribute now.
,
Dec 12 2016
Working on Element#assignedSlot and Element nullable
,
Dec 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c4308a50c6dd3324293fb419a2762e9ae4c5e84b commit c4308a50c6dd3324293fb419a2762e9ae4c5e84b Author: lunalu <lunalu@chromium.org> Date: Mon Dec 12 15:04:45 2016 Made Text#assignedSlot and Element#assignedSlot nullable. Also changed the order of the attributes/methods under Shadow DOM to match the spec. BUG=662005 Review-Url: https://codereview.chromium.org/2562343002 Cr-Commit-Position: refs/heads/master@{#437873} [modify] https://crrev.com/c4308a50c6dd3324293fb419a2762e9ae4c5e84b/third_party/WebKit/Source/core/dom/Element.idl [modify] https://crrev.com/c4308a50c6dd3324293fb419a2762e9ae4c5e84b/third_party/WebKit/Source/core/dom/Text.idl
,
Dec 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1dc57917e3d0d9a4a15f78bebeae831b221d80d1 commit 1dc57917e3d0d9a4a15f78bebeae831b221d80d1 Author: lunalu <lunalu@chromium.org> Date: Tue Dec 13 09:12:01 2016 Made nextElementSibling and previousElementSibling nullable No generated code is changed spec: https://dom.spec.whatwg.org/#interface-nondocumenttypechildnode BUG=662005 Review-Url: https://codereview.chromium.org/2570433004 Cr-Commit-Position: refs/heads/master@{#438105} [modify] https://crrev.com/1dc57917e3d0d9a4a15f78bebeae831b221d80d1/third_party/WebKit/Source/core/dom/NonDocumentTypeChildNode.idl
,
Dec 13 2016
markdittmer@, if the tool could output something terse like this it'd be great for lists like the above: Document#documentURI (is nullable, shouldn't be) HTMLTextAreaElement#selectionStart (is not nullable, should be) RTCPeerConnection#createDataChannel(label) (is nullable, shouldn't be) Pretty-printing methods so that the argument is in the right place isn't really important, but being able to sort and group by interface would be valuable.
,
Dec 13 2016
(Ultimately, output on compiler warning/error format with some context would be cool, or just a view of the IDL text itself with the problems in the margins or between lines.)
,
Dec 13 2016
Below is the list without the type The following interfaces have nullable arguments mismatching the spec: AudioBufferSourceNode > buffer BluetoothDevice > gatt CSSStyleDeclaration > setProperty > priority CSSStyleDeclaration > setProperty > value CharacterData > nextElementSibling CharacterData > previousElementSibling ClipboardEvent > clipboardData DataTransferItem > webkitGetAsEntry Document > documentURI DragEvent > dataTransfer Element > nextElementSibling Element > previousElementSibling ElementDefinitionOptions > extends FetchEvent > clientId FetchEventInit > clientId GamepadEventInit > gamepad HTMLAllCollection HTMLInputElement > selectionDirection HTMLInputElement > selectionEnd HTMLInputElement > selectionStart HTMLTextAreaElement > selectionDirection HTMLTextAreaElement > selectionEnd HTMLTextAreaElement > selectionStart History > pushState > title History > replaceState > title ImageBitmapOptions > resizeHeight ImageBitmapOptions > resizeWidth ImageCapture > setOptions > photoSettings InputEvent > data InputEventInit > data IntersectionObserverEntry > rootBounds MIDIConnectionEventInit > port MIDIMessageEventInit > data MIDIPort > manufacturer MIDIPort > name MIDIPort > version MediaList > mediaText MessageEvent > ports MimeTypeArray > item MimeTypeArray > namedItem Navigator > language NavigatorLanguage > language NavigatorUserMediaError > constraintName Node > baseURI NonDocumentTypeChildNode > nextElementSibling NonDocumentTypeChildNode > previousElementSibling OscillatorOptions > periodicWave Path2D > addPath > transform Plugin > item Plugin > namedItem PluginArray > item PluginArray > namedItem PresentationConnection > id ProcessingInstruction > sheet PushEvent > data RTCDataChannel > maxRetransmits RTCIceCandidate > sdpMLineIndex RTCIceCandidate > sdpMid RTCIceCandidateInit > sdpMLineIndex RTCIceCandidateInit > sdpMid RTCPeerConnection > addIceCandidate > candidate RTCPeerConnection > createDataChannel > label RTCSessionDescription > sdp RTCSessionDescription > type Response > body SpeechRecognitionEvent > emma SpeechRecognitionEvent > interpretation SpeechRecognitionEvent > results SpeechSynthesisUtterance > voice USBInterface > alternate VRDisplay > stageParameters VRDisplayEvent > reason VRDisplayEventInit > display WebGL2RenderingContext > createQuery WebGL2RenderingContext > createSampler WebGL2RenderingContext > createTransformFeedback WebGL2RenderingContext > createVertexArray WebGL2RenderingContext > fenceSync WebGL2RenderingContext > getQuery WebGL2RenderingContext > getTransformFeedbackVarying WebGL2RenderingContextBase > bufferData > srcData WebGL2RenderingContextBase > createQuery WebGL2RenderingContextBase > createSampler WebGL2RenderingContextBase > createTransformFeedback WebGL2RenderingContextBase > createVertexArray WebGL2RenderingContextBase > fenceSync WebGL2RenderingContextBase > getQuery WebGL2RenderingContextBase > getTransformFeedbackVarying WebGL2RenderingContextBase > readPixels > dstData Window > parent Window > top WorkerNavigator > language
,
Dec 14 2016
,
Dec 14 2016
,
Dec 14 2016
,
Dec 14 2016
,
Dec 14 2016
,
Dec 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8734aac57d4f724768a53326542d911f1836ad45 commit 8734aac57d4f724768a53326542d911f1836ad45 Author: lunalu <lunalu@chromium.org> Date: Wed Dec 14 13:11:55 2016 Low risk non-nullable => nullable change Made the following arguments / attributes nullable to match the spec. BUG=662005 Review-Url: https://codereview.chromium.org/2569993003 Cr-Commit-Position: refs/heads/master@{#438498} [modify] https://crrev.com/8734aac57d4f724768a53326542d911f1836ad45/third_party/WebKit/Source/core/events/ClipboardEvent.idl [modify] https://crrev.com/8734aac57d4f724768a53326542d911f1836ad45/third_party/WebKit/Source/core/events/DragEvent.idl [modify] https://crrev.com/8734aac57d4f724768a53326542d911f1836ad45/third_party/WebKit/Source/core/frame/Window.idl [modify] https://crrev.com/8734aac57d4f724768a53326542d911f1836ad45/third_party/WebKit/Source/modules/filesystem/DataTransferItemFileSystem.idl
,
Dec 14 2016
,
Dec 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9a86e1807f73676102d1b6cb4d660604390d3e76 commit 9a86e1807f73676102d1b6cb4d660604390d3e76 Author: lunalu <lunalu@chromium.org> Date: Wed Dec 14 18:42:00 2016 Low risk nullable => non-nullable change Made the following arguments / attributes non-nullable to match the spec. History#pushState argument title seemed to be never used by our code. https://github.com/whatwg/html/issues/2174 BUG=662005 Review-Url: https://codereview.chromium.org/2569663005 Cr-Commit-Position: refs/heads/master@{#438548} [modify] https://crrev.com/9a86e1807f73676102d1b6cb4d660604390d3e76/third_party/WebKit/Source/core/frame/History.idl [modify] https://crrev.com/9a86e1807f73676102d1b6cb4d660604390d3e76/third_party/WebKit/Source/core/imagebitmap/ImageBitmapOptions.idl
,
Dec 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/49512757a40ff27cc36b9a92dd09ac8c14687909 commit 49512757a40ff27cc36b9a92dd09ac8c14687909 Author: lunalu <lunalu@chromium.org> Date: Wed Dec 14 19:05:41 2016 Make item getter and nameditem getter return nullable in Plugin and PluginArray Spec: https://html.spec.whatwg.org/multipage/webappapis.html#plugins-2 BUG=662005 Review-Url: https://codereview.chromium.org/2575893002 Cr-Commit-Position: refs/heads/master@{#438568} [modify] https://crrev.com/49512757a40ff27cc36b9a92dd09ac8c14687909/third_party/WebKit/Source/modules/plugins/Plugin.idl [modify] https://crrev.com/49512757a40ff27cc36b9a92dd09ac8c14687909/third_party/WebKit/Source/modules/plugins/PluginArray.idl
,
Dec 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6ea094398a321f2d36aa82fd52087f40d64df651 commit 6ea094398a321f2d36aa82fd52087f40d64df651 Author: lunalu <lunalu@chromium.org> Date: Wed Dec 14 19:53:39 2016 Fix low risk trivial non-nullable => nullable idl mismatch in WebGL2RenderingContextBase.idl https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7 create*() methods, fenceSync() method, and getTransformFeedbackVarying() method cause no change in generated files BUG=662005 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2568743009 Cr-Commit-Position: refs/heads/master@{#438601} [modify] https://crrev.com/6ea094398a321f2d36aa82fd52087f40d64df651/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.idl
,
Dec 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f35734e8595bf59e62f8b8b87c0dd638bd01e472 commit f35734e8595bf59e62f8b8b87c0dd638bd01e472 Author: lunalu <lunalu@chromium.org> Date: Thu Dec 15 17:48:05 2016 Fix low risk non-nullable => nullable attributes / arguments to match the spec BUG=662005 Review-Url: https://codereview.chromium.org/2574303002 Cr-Commit-Position: refs/heads/master@{#438869} [modify] https://crrev.com/f35734e8595bf59e62f8b8b87c0dd638bd01e472/third_party/WebKit/Source/core/dom/ProcessingInstruction.idl [modify] https://crrev.com/f35734e8595bf59e62f8b8b87c0dd638bd01e472/third_party/WebKit/Source/modules/imagecapture/ImageCapture.idl [modify] https://crrev.com/f35734e8595bf59e62f8b8b87c0dd638bd01e472/third_party/WebKit/Source/modules/presentation/PresentationConnection.idl [modify] https://crrev.com/f35734e8595bf59e62f8b8b87c0dd638bd01e472/third_party/WebKit/Source/modules/push_messaging/PushEvent.idl
,
Dec 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3d59b1cb65c0b2566316b0886e69e49ef7589d6e commit 3d59b1cb65c0b2566316b0886e69e49ef7589d6e Author: lunalu <lunalu@chromium.org> Date: Fri Dec 16 15:33:55 2016 Fix low risk nullable => non nullable change for NavigatorLanguage.language NavigatorLanguage::language() returns defaultLanguage() which either preferredLanguagesOverride()[0] if preferredLanguagesOverride() is not empty or platformLanguage() which will never return null; therefore, the change is safe. Added web platform test for InputEvent BUG=662005 Review-Url: https://codereview.chromium.org/2570203004 Cr-Commit-Position: refs/heads/master@{#439111} [modify] https://crrev.com/3d59b1cb65c0b2566316b0886e69e49ef7589d6e/third_party/WebKit/LayoutTests/imported/wpt/MANIFEST.json [add] https://crrev.com/3d59b1cb65c0b2566316b0886e69e49ef7589d6e/third_party/WebKit/LayoutTests/imported/wpt/uievents/constructors/inputevent-constructor.html [modify] https://crrev.com/3d59b1cb65c0b2566316b0886e69e49ef7589d6e/third_party/WebKit/Source/core/frame/NavigatorLanguage.idl
,
Apr 14 2017
,
Jun 12 2017
,
Nov 1 2017
|
|||||||||||||
►
Sign in to add a comment |
||||||||||||||||
Comment 1 by foolip@chromium.org
, Nov 3 2016