From 28b8cb34d3d37c4adbe24efb19e49ed3ef1fc357 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Tue, 25 Aug 2020 23:48:43 +0000 Subject: [PATCH] [dart:html] Unify shared attribute metadata from MDN browser-compat-data includes info on elements as both part of the Web API and the individual tagged elements. The scripts would default to whatever was processed last instead of trying to unify any conflicts that may come up with the different sources of truth. This CL addresses that issue by choosing the "stricter" of the two versions. Change-Id: Id3437c14276faf8af3fe41d7a6eb83defc9bd0b0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/155844 Commit-Queue: Srujan Gaddam Reviewed-by: Sigmund Cherem --- sdk/lib/html/dart2js/html_dart2js.dart | 48 ++--- sdk/lib/svg/dart2js/svg_dart2js.dart | 4 +- ...ocument_register_type_extensions_test.dart | 2 +- tests/lib/html/url_test.dart | 2 +- tools/dom/scripts/mdnreader.py | 197 +++++++++++++++--- 5 files changed, 198 insertions(+), 55 deletions(-) diff --git a/sdk/lib/html/dart2js/html_dart2js.dart b/sdk/lib/html/dart2js/html_dart2js.dart index 4842c307884..9d1ebd605e4 100644 --- a/sdk/lib/html/dart2js/html_dart2js.dart +++ b/sdk/lib/html/dart2js/html_dart2js.dart @@ -1937,13 +1937,13 @@ class ButtonElement extends HtmlElement { set formAction(String? value) native; - String get formEnctype native; + String? get formEnctype native; - set formEnctype(String value) native; + set formEnctype(String? value) native; - String get formMethod native; + String? get formMethod native; - set formMethod(String value) native; + set formMethod(String? value) native; bool get formNoValidate native; @@ -2120,15 +2120,15 @@ class CanvasElement extends HtmlElement implements CanvasImageSource { /// The height of this canvas element in CSS pixels. - int get height native; + int? get height native; - set height(int value) native; + set height(int? value) native; /// The width of this canvas element in CSS pixels. - int get width native; + int? get width native; - set width(int value) native; + set width(int? value) native; MediaStream captureStream([num? frameRate]) native; @@ -15994,9 +15994,9 @@ class FieldSetElement extends HtmlElement { */ FieldSetElement.created() : super.created(); - bool get disabled native; + bool? get disabled native; - set disabled(bool value) native; + set disabled(bool? value) native; @Returns('HtmlCollection') @Creates('HtmlCollection') @@ -21740,34 +21740,34 @@ class MeterElement extends HtmlElement { /// Checks if this type is supported on the current platform. static bool get supported => Element.isTagSupported('meter'); - num get high native; + num? get high native; - set high(num value) native; + set high(num? value) native; @Unstable() @Returns('NodeList') @Creates('NodeList') List? get labels native; - num get low native; + num? get low native; - set low(num value) native; + set low(num? value) native; - num get max native; + num? get max native; - set max(num value) native; + set max(num? value) native; - num get min native; + num? get min native; - set min(num value) native; + set min(num? value) native; - num get optimum native; + num? get optimum native; - set optimum(num value) native; + set optimum(num? value) native; - num get value native; + num? get value native; - set value(num value) native; + set value(num? value) native; } // Copyright (c) 2012, the Dart project authors. Please see the AUTHORS file // for details. All rights reserved. Use of this source code is governed by a @@ -29170,9 +29170,9 @@ class StyleElement extends HtmlElement { StyleSheet? get sheet native; - String get type native; + String? get type native; - set type(String value) native; + set type(String? value) native; } // Copyright (c) 2012, the Dart project authors. Please see the AUTHORS file // for details. All rights reserved. Use of this source code is governed by a diff --git a/sdk/lib/svg/dart2js/svg_dart2js.dart b/sdk/lib/svg/dart2js/svg_dart2js.dart index f016890f23b..aa97d1981a7 100644 --- a/sdk/lib/svg/dart2js/svg_dart2js.dart +++ b/sdk/lib/svg/dart2js/svg_dart2js.dart @@ -683,9 +683,9 @@ class FEColorMatrixElement extends SvgElement AnimatedString get in1 native; - AnimatedEnumeration get type native; + AnimatedEnumeration? get type native; - AnimatedNumberList get values native; + AnimatedNumberList? get values native; // From SVGFilterPrimitiveStandardAttributes diff --git a/tests/lib/html/custom/document_register_type_extensions_test.dart b/tests/lib/html/custom/document_register_type_extensions_test.dart index bbeafb56283..7da7cfdf1ad 100644 --- a/tests/lib/html/custom/document_register_type_extensions_test.dart +++ b/tests/lib/html/custom/document_register_type_extensions_test.dart @@ -60,7 +60,7 @@ class MyCanvas extends CanvasElement { CanvasRenderingContext2D context = this.getContext('2d') as CanvasRenderingContext2D; context.fillStyle = 'red'; - context.fillRect(0, 0, width, height); + context.fillRect(0, 0, width!, height!); context.fill(); var data = context.getImageData(0, 0, 1, 1).data; diff --git a/tests/lib/html/url_test.dart b/tests/lib/html/url_test.dart index 201f52386b8..857f8ae29f1 100644 --- a/tests/lib/html/url_test.dart +++ b/tests/lib/html/url_test.dart @@ -17,7 +17,7 @@ Blob createImageBlob() { var context = canvas.context2D; context.fillStyle = 'red'; - context.fillRect(0, 0, canvas.width, canvas.height); + context.fillRect(0, 0, canvas.width!, canvas.height!); var dataUri = canvas.toDataUrl('image/png'); var byteString = window.atob(dataUri.split(',')[1]); diff --git a/tools/dom/scripts/mdnreader.py b/tools/dom/scripts/mdnreader.py index b748d1ff850..9fe43a3e977 100644 --- a/tools/dom/scripts/mdnreader.py +++ b/tools/dom/scripts/mdnreader.py @@ -5,8 +5,15 @@ import json import os.path +import re import sys +_COMPAT_KEY = '__compat' +_EXPERIMENTAL_KEY = 'experimental' +_STATUS_KEY = 'status' +_SUPPORT_KEY = 'support' +_VERSION_ADDED_KEY = 'version_added' + def _get_browser_compat_data(): current_dir = os.path.dirname(__file__) @@ -89,17 +96,164 @@ def _get_browser_compat_data(): } if interface in browser_compat_data: - browser_compat_data[interface].update(metadata) + _unify_metadata(browser_compat_data[interface], + metadata) else: browser_compat_data[interface] = metadata else: names[:] = [] # Do not go underneath + # Attempts to unify two compatibility infos by taking the union of both, and + # for conflicting information, taking the "stricter" of the two versions. + # Updates `a` in place to represent the union of `a` and `b`. + def _unify_compat(a, b): + + def _has_compat_data(metadata): + return _COMPAT_KEY in metadata and _SUPPORT_KEY in metadata[_COMPAT_KEY] + + # Unifies the support statements of both metadata and updates + # `support_a` in place. If either metadata do not contain simple support + # statements, defaults attribute to not supported. + def _unify_support(support_a, support_b): + for browser in support_a.keys(): + if browser in support_b: + if _is_simple_support_statement(support_a[browser]) and _is_simple_support_statement(support_b[browser]): + support_a[browser][_VERSION_ADDED_KEY] = _unify_versions( + support_a[browser][_VERSION_ADDED_KEY], + support_b[browser][_VERSION_ADDED_KEY]) + else: + # Only support simple statements for now. + support_a[browser] = {_VERSION_ADDED_KEY: None} + for browser in support_b.keys(): + if not browser in support_a: + support_a[browser] = support_b[browser] + + if not _has_compat_data(b): + return + if not _has_compat_data(a): + a[_COMPAT_KEY] = b[_COMPAT_KEY] + return + + support_a = a[_COMPAT_KEY][_SUPPORT_KEY] + support_b = b[_COMPAT_KEY][_SUPPORT_KEY] + + _unify_support(support_a, support_b) + + # Unifies any status info in the two metadata. Modifies `a` in place to + # represent the union of both `a` and `b`. + def _unify_status(a, b): + + def _has_status(metadata): + return _COMPAT_KEY in metadata and _STATUS_KEY in metadata[_COMPAT_KEY] + + # Modifies `status_a` in place to combine "experimental" tags. + def _unify_experimental(status_a, status_b): + # If either of the statuses report experimental, assume attribute is + # experimental. + status_a[_EXPERIMENTAL_KEY] = status_a.get( + _EXPERIMENTAL_KEY, False) or status_b.get(_EXPERIMENTAL_KEY, False) + + if not _has_status(b): + return + if not _has_status(a): + a[_COMPAT_KEY] = b[_COMPAT_KEY] + return + + status_a = a[_COMPAT_KEY][_STATUS_KEY] + status_b = b[_COMPAT_KEY][_STATUS_KEY] + + _unify_experimental(status_a, status_b) + + # If there exists multiple definitions of the same interface metadata e.g. + # elements, this attempts to unify the compatibilities for the interface as + # well as for each attribute. + def _unify_metadata(a, b): + # Unify the compatibility statement and status of the API or element. + _unify_compat(a, b) + _unify_status(a, b) + # Unify the compatibility statement and status of each attribute. + for attr in list(a.keys()): + if attr == _COMPAT_KEY: + continue + if attr in b: + _unify_compat(a[attr], b[attr]) + _unify_status(a[attr], b[attr]) + for attr in b.keys(): + if not attr in a: + a[attr] = b[attr] + os.path.walk(browser_compat_folder, visitor, browser_compat_folder) return browser_compat_data +# Given two version values for a given browser, chooses the more strict version. +def _unify_versions(version_a, version_b): + # Given two valid version strings, compares parts of the version string + # iteratively. + def _greater_version(version_a, version_b): + version_a_split = map(int, version_a.split('.')) + version_b_split = map(int, version_b.split('.')) + for i in range(min(len(version_a_split), len(version_b_split))): + if version_a_split[i] > version_b_split[i]: + return version_a + elif version_a_split[i] < version_b_split[i]: + return version_b + return version_a if len(version_a_split) > len( + version_b_split) else version_b + + # Validate that we can handle the given version. + def _validate_version(version): + if not version: + return False + if version is True: + return True + if isinstance(version, str) or isinstance(version, unicode): + pattern = re.compile('^([0-9]+\.)*[0-9]+$') + if not pattern.match(version): + # It's possible for version strings to look like '<35'. We don't + # attempt to parse the conditional logic, and just default to + # potentially incompatible. + return None + return version + else: + raise ValueError( + 'Type of version_a was not handled correctly! type(version) = ' + + str(type(version))) + + version_a = _validate_version(version_a) + version_b = _validate_version(version_b) + # If one version reports not supported, default to not supported. + if not version_a or not version_b: + return False + # If one version reports always supported, the other version can only be + # more strict. + if version_a is True: + return version_b + if version_b is True: + return version_a + + return _greater_version(version_a, version_b) + + +# At this time, we only handle simple support statements due to the complexity +# and variability around support statements with multiple elements. +def _is_simple_support_statement(support_statement): + if isinstance(support_statement, list): # array_support_statement + # TODO(srujzs): Parse this list to determine compatibility. Will + # likely require parsing for 'version_removed' keys. Notes about + # which browser version enabled this attribute for which + # platform also complicates things. For now, we assume it's not + # compatible. + return False + if len(support_statement.keys()) > 1: + # If it's anything more complicated than 'version_added', like + # 'notes' that specify platform versions, we assume it's not + # compatible. + return False + return True + + class MDNReader(object): # Statically initialize and treat as constant. _BROWSER_COMPAT_DATA = _get_browser_compat_data() @@ -114,40 +268,29 @@ class MDNReader(object): # For now, we will require support for browsers since the last IDL roll. # TODO(srujzs): Determine if this is too conservative. browser_version_map = { - 'chrome': 63, - 'firefox': 57, - 'safari': 11, + 'chrome': '63', + 'firefox': '57', + 'safari': '11', # We still support the latest version of IE. - 'ie': 11, - 'opera': 50, + 'ie': '11', + 'opera': '50', } - version_key = 'version_added' for browser in browser_version_map.keys(): - support_data = compat_data['support'] + support_data = compat_data[_SUPPORT_KEY] if browser not in support_data: return False support_statement = support_data[browser] - if isinstance(support_statement, list): # array_support_statement - # TODO(srujzs): Parse this list to determine compatibility. Will - # likely require parsing for 'version_removed' keys. Notes about - # which browser version enabled this attribute for which - # platform also complicates things. For now, we assume it's not - # compatible. + if not _is_simple_support_statement(support_statement): return False - if len(support_statement.keys()) > 1: - # If it's anything more complicated than 'version_added', like - # 'notes' that specify platform versions, we assume it's not - # compatible. - return False - version = support_statement[version_key] - if not version or browser_version_map[browser] < float(version): - # simple_support_statement + version = support_statement[_VERSION_ADDED_KEY] + # Compare version strings, target should be the more strict version. + target = browser_version_map[browser] + if _unify_versions(version, target) != target: return False + # If the attribute is experimental, we assume it's not compatible. - status_data = compat_data['status'] - experimental_key = 'experimental' - if experimental_key in status_data and \ - status_data[experimental_key]: + status_data = compat_data[_STATUS_KEY] + if _EXPERIMENTAL_KEY in status_data and status_data[_EXPERIMENTAL_KEY]: return False return True @@ -169,7 +312,7 @@ class MDNReader(object): return False elif id_name in interface_dict: id_data = interface_dict[id_name] - return self._get_attr_compatibility(id_data['__compat']) + return self._get_attr_compatibility(id_data[_COMPAT_KEY]) else: # Might be an attribute that is defined in a parent interface. # We defer until attribute emitting to determine if this is the