Issue 11679007: Adding support checks for partially supported element types and updating tests.

Issue 11679007: Adding support checks for partially supported element types and updating tests. (Closed)

Created:
7 years, 12 months ago by blois

Modified:
7 years, 12 months ago

Reviewers:
Emily Fortuna

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Adding 'supported' checks to elements which are not supported on all platforms and adding annotations as to which platforms they are supported on. Also refactoring tests to accurately reflect what is and is not supported on the various platforms. BUG= Committed: https://code.google.com/p/dart/source/detail?r=16553

Patch Set 1 #

Patch Set 2 : Adding annotations. #

Patch Set 3 : #

Patch Set 4 : Removing duplicated contentelement_test. #

Total comments: 8

Patch Set 5 : #

Created: 7 years, 12 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+616 lines, -249 lines)			Patch
M	sdk/lib/html/dart2js/html_dart2js.dart	View	1 2	14 chunks	+101 lines, -1 line	0 comments	Download
M	sdk/lib/html/dartium/html_dartium.dart	View		15 chunks	+75 lines, -6 lines	0 comments	Download
M	sdk/lib/html/scripts/generator.py	View	1	2 chunks	+54 lines, -3 lines	0 comments	Download
M	sdk/lib/html/scripts/htmldartgenerator.py	View	1	2 chunks	+11 lines, -0 lines	0 comments	Download
M	sdk/lib/html/scripts/systemhtml.py	View	1 2 3 4	3 chunks	+24 lines, -0 lines	0 comments	Download
M	sdk/lib/html/scripts/systemnative.py	View	1	2 chunks	+9 lines, -0 lines	0 comments	Download
M	sdk/lib/html/templates/html/impl/impl_Element.darttemplate	View		3 chunks	+15 lines, -1 line	0 comments	Download
M	sdk/lib/indexed_db/dart2js/indexed_db_dart2js.dart	View	1	1 chunk	+3 lines, -3 lines	0 comments	Download
M	sdk/lib/indexed_db/dartium/indexed_db_dartium.dart	View	1	1 chunk	+3 lines, -3 lines	0 comments	Download
D	tests/html/contentelement_test.dart	View	1 2 3	1 chunk	+0 lines, -20 lines	0 comments	Download
M	tests/html/element_test.dart	View	1 2 3 4	3 chunks	+0 lines, -179 lines	0 comments	Download
A	tests/html/element_types_test.dart	View		1 chunk	+300 lines, -0 lines	0 comments	Download
M	tests/html/element_webkit_test.dart	View	1	2 chunks	+0 lines, -27 lines	0 comments	Download
M	tests/html/html.status	View	1 2 3	7 chunks	+21 lines, -6 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Emily Fortuna

https://codereview.chromium.org/11679007/diff/6001/sdk/lib/html/scripts/systemhtml.py File sdk/lib/html/scripts/systemhtml.py (right): https://codereview.chromium.org/11679007/diff/6001/sdk/lib/html/scripts/systemhtml.py#newcode76 sdk/lib/html/scripts/systemhtml.py:76: "&& (new Element.tag('keygen') is KeygenElement)", can we programmatically determine ...

7 years, 12 months ago (2012-12-27 23:06:49 UTC) #2

blois

7 years, 12 months ago (2012-12-28 00:43:27 UTC) #3

https://codereview.chromium.org/11679007/diff/6001/sdk/lib/html/scripts/syste...
File sdk/lib/html/scripts/systemhtml.py (right):

https://codereview.chromium.org/11679007/diff/6001/sdk/lib/html/scripts/syste...
sdk/lib/html/scripts/systemhtml.py:76: "&& (new Element.tag('keygen') is
KeygenElement)",
On 2012/12/27 23:06:49, Emily Fortuna wrote:
> can we programmatically determine for which classes if we need to just check
> "Element.isTagSupported('keygen')" versus "Element.isTagSupported('keygen') 
&&
> (new Element.tag('keygen') is KeygenElement)" ?  so we don't have to
explicitly
> write all of this out?

isTagSupported checks if the browser treats it as an UnknownElement or not,
which is correct for the general case. But in some cases, the browser returns a
different element type than we ask for- this is what the additional check is.

If we had first-class generics then it'd be easier. As it is, the constructors
will generate exceptions in checked mode when the browser returns the wrong
element type, and that's what this table is based off of.

https://codereview.chromium.org/11679007/diff/6001/sdk/lib/html/scripts/syste...
sdk/lib/html/scripts/systemhtml.py:496: 
On 2012/12/27 23:06:49, Emily Fortuna wrote:
> remove extra newline

Done.

https://codereview.chromium.org/11679007/diff/6001/tests/html/element_test.dart
File tests/html/element_test.dart (right):

https://codereview.chromium.org/11679007/diff/6001/tests/html/element_test.da...
tests/html/element_test.dart:48: void testConstructorHelper(String tag, String
htmlSnippet,
On 2012/12/27 23:06:49, Emily Fortuna wrote:
> this function is no longer called in this test. Remove?

Done.

https://codereview.chromium.org/11679007/diff/6001/tests/html/element_types_t...
File tests/html/element_types_test.dart (right):

https://codereview.chromium.org/11679007/diff/6001/tests/html/element_types_t...
tests/html/element_types_test.dart:75: expect((new AnchorElement()) is
AnchorElement, true);
On 2012/12/27 23:06:49, Emily Fortuna wrote:
> this is no longer testing constructing an element from an html snippet like in
> testConstructorHelper in element_test.dart. Is this intentional?

Yeah, basically the old way tested the HTML parse rules for elements, which are
considerably more complex than creating tags. This is great if we're generating
HTML and want to validate that it works across browsers, but for our uses it
introduces a lot more variability than necessary.

Some elements allow content, some do not, some defaulted to having content while
others did not, etc.

reasons sgtm.
lgtm.

Expand Messages | Collapse Messages