Custom Elements: layout tests for custom built-in elements definition

third_party/WebKit/LayoutTests/custom-elements/spec/define-element.html

Index: third_party/WebKit/LayoutTests/custom-elements/spec/define-element.html
diff --git a/third_party/WebKit/LayoutTests/custom-elements/spec/define-element.html b/third_party/WebKit/LayoutTests/custom-elements/spec/define-element.html
index ca3c310d29f87fd57cb8ce96fa2cb44683df5cdc..9185c40500062b838dd5d5faa589e97b4cfb1141 100644
--- a/third_party/WebKit/LayoutTests/custom-elements/spec/define-element.html
+++ b/third_party/WebKit/LayoutTests/custom-elements/spec/define-element.html
@@ -102,6 +102,51 @@ promise_test((t) => {
 }, 'HTMLElement constructor looks up definitions in the current global-' +
    'reused constructor');
 
+// TODO: What error should be thrown if extends option not specified or null?

[dominicc (has gone to gerrit), 2016/09/12 05:12:54]

Part of it is element definition [1] step 6. If the option is not specified then
_extends_ is null. Step 7 skips stuff if that's the case.

Then there's the question of what to do if extends is specified, but null or
undefined. This is a bit more subtle. Because ElementRegistrationOptions is a
Dictionary a lot of its handling is specified in Web IDL, especially the
EcmaScript binding [2]. You see the values are copied out of the argument into
the dictionary in step 5.1.3.2, but undefined and null are filtered in step
5.1.2-3.

So null and undefined also end up missing from the dictionary and step 7 is
skipped for them, too.

[1] https://html.spec.whatwg.org/#element-definition
[2] https://heycam.github.io/webidl/#es-dictionary

+
+test_with_window((w) => {
+  class A extends w.HTMLButtonElement {}
+  let invalid_values = [

[kojii, 2016/09/12 04:52:23]

Let's make the variable name more explicit; in this case this is a list of
"valid custom element name", correct?

+    'a-a',
+    'z0-y0',
+    'emotion-\u1f60d',
+    'math-\u03b1',
+    'a.b-c'
+  ];
+  invalid_values.forEach((val) => {
+    assert_throws_dom_exception(w, 'NotSupportedError', () => {
+      w.customElements.define('a-a', A, { extends: val });
+    }, 'extends value must not be valid customized element name')

[dominicc (has gone to gerrit), 2016/09/12 05:12:54]

It might be good to include the actual value in the failure message to make it
easy to understand which case is failing.

+  });
+}, 'Invalid extends value');

[dominicc (has gone to gerrit), 2016/09/12 05:12:54]

This and the next test may show up in output like

PASS Invalid extends value
FAIL Invalid extends value

which isn't very illuminating. They're different tests, so briefly describe the
difference: mention potential custom element names and names of elements whose
interface is HTMLUnknown element so people looking at the test output have an
high-level idea what each test is doing.

+
+test_with_window((w) => {
+  class A extends w.HTMLButtonElement {}
+  let invalid_values = [

[kojii, 2016/09/12 04:52:23]

ditto; such as "element_names_for_HTMLUnknownElement" or such.

+    'bgsound',
+    'blink',
+    'isindex',
+    'multicol',
+    'nextid',
+    'spacer',
+    42
+  ]
+  invalid_values.forEach((val) => {
+    assert_throws_dom_exception(w, 'NotSupportedError', () => {
+      w.customElements.define('a-a', A, { extends: val });
+    }, 'extends value cannot be HTMLUnknownElement');

[dominicc (has gone to gerrit), 2016/09/12 05:12:55]

Make this more precise; the value isn't HTMLUnknownElement but a string; I think
the spec text calls the thing you're interested in the "element interface for
extends"? Briefly paraphrase it for the test.

BTW it's a common challenge in these tests to be testing some concept in the
spec that has a different name or is only available implicitly in the API. Try
to balance brevity, precision, readability. When all else fails cutting and
pasting lines from the spec to cite it directly, and make variable names match
spec terms, is a safe way to go.

+  });
+}, 'Invalid extends value');
+
+test_with_window((w) => {
+  class A extends w.HTMLButtonElement {}
+  w.customElements.define('defined-name', A, { extends: 'button'});

[dominicc (has gone to gerrit), 2016/09/12 05:12:54]

Be consistent with spacing; you put a space before } above but not here.

It may be more succinct without spaces after {/before } for object literals.

+  assert_equals(new A().localName, 'button', 'extends value should == local ' +

[kojii, 2016/09/12 04:52:23]

Let's make human text; such as "localName should be extends".

[dominicc (has gone to gerrit), 2016/09/12 05:12:54]

Use English, so not == but just 'be' or something like that.

When wrapping, wrap at a higher-level expression if it results in the same
number of line breaks. In this case, you can probably put the whole string on
one line by wrapping after , and then you don't need the ' + ' either which is
simpler anyway.

+    'name');
+  assert_not_equals(new A().localName, 'defined-name', 'extends value ' +

[kojii, 2016/09/12 04:52:23]

ditto.

+    'should != defined-name');
+}, 'Localname should be extends value not defined-name');

[dominicc (has gone to gerrit), 2016/09/12 05:12:54]

Write 'Local name ...' with a space. localName is OK when you need to refer to
that specific API, but then match the case.

+
 promise_test((t) => {
   return Promise.all([create_window_in_test(t), create_window_in_test(t)])
   .then(([w1, w2]) => {