Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(396)

Issue 2477713003: Custom Elements: Check Definition in createElement, Create Customized Built-in Elements Sync (Closed)

Created:
4 years, 1 month ago by yurak
Modified:
4 years, 1 month ago
CC:
blink-reviews, blink-reviews-dom_chromium.org, chromium-reviews, dglazkov+blink, dominicc+watchlist_chromium.org, eae+blinkwatch, rwlbuis, sof, webcomponents-bugzilla_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

https://dom.spec.whatwg.org/#dom-document-createelement Definition is checked for HTML namespace elements in document.createElement Customized built-in elements can now be created with the document.createElement NOTE: This is behind runtime enabled flag "CustomElementsBuiltin". Syntax: https://html.spec.whatwg.org/multipage/scripting.html#custom-elements-customized-builtin-example Implemented create element algorithm https://dom.spec.whatwg.org/#concept-create-element Steps 5-7: /core/dom/custom/CustomElement.cpp Added some tests. (Sorry this patch is big) BUG=648828, 657583 Committed: https://crrev.com/fe200cd217b99368dbded68a0350d33c552c6a5b Cr-Commit-Position: refs/heads/master@{#433146}

Patch Set 1 #

Patch Set 2 : createElement - sync #

Total comments: 8

Patch Set 3 : Made changes #

Total comments: 2

Patch Set 4 : New tests #

Patch Set 5 : New tests #

Total comments: 4

Patch Set 6 : V1 definition check in document::createElement #

Patch Set 7 : V1 definition check in document::createElement #

Total comments: 8

Patch Set 8 : More tests, fix case conversion of 'is' attribute #

Patch Set 9 : More tests, fix case conversion of 'is' attribute #

Total comments: 3

Patch Set 10 : V0-V1 interoperability #

Total comments: 1

Patch Set 11 : V0-V1 interoperability #

Patch Set 12 : patch fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+308 lines, -57 lines) Patch
A third_party/WebKit/LayoutTests/custom-elements/spec/create-element.html View 1 2 3 4 5 6 7 1 chunk +87 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/custom-elements/spec/create-element-defined-synchronous.html View 1 2 3 4 5 1 chunk +43 lines, -8 lines 0 comments Download
M third_party/WebKit/LayoutTests/custom-elements/v0-v1-interop.html View 1 2 3 4 5 1 chunk +29 lines, -0 lines 0 comments Download
D third_party/WebKit/LayoutTests/fast/dom/custom/create-element-second-arg.html View 1 2 3 4 5 1 chunk +0 lines, -16 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Document.cpp View 1 2 3 4 5 6 7 8 9 10 11 5 chunks +105 lines, -20 lines 0 comments Download
M third_party/WebKit/Source/core/dom/custom/CustomElement.h View 1 chunk +8 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/dom/custom/CustomElement.cpp View 1 2 3 4 4 chunks +36 lines, -10 lines 0 comments Download

Messages

Total messages: 28 (14 generated)
dominicc (has gone to gerrit)
Feedback inline. https://codereview.chromium.org/2477713003/diff/20001/third_party/WebKit/Source/core/dom/Document.cpp File third_party/WebKit/Source/core/dom/Document.cpp (right): https://codereview.chromium.org/2477713003/diff/20001/third_party/WebKit/Source/core/dom/Document.cpp#newcode731 third_party/WebKit/Source/core/dom/Document.cpp:731: int builtin = 0; Use bool, not ...
4 years, 1 month ago (2016-11-07 01:59:00 UTC) #4
dominicc (has gone to gerrit)
This is very close. Just needs a few tweaks and some tests. https://codereview.chromium.org/2477713003/diff/40001/third_party/WebKit/Source/core/dom/Document.cpp File third_party/WebKit/Source/core/dom/Document.cpp ...
4 years, 1 month ago (2016-11-08 08:36:29 UTC) #5
dominicc (has gone to gerrit)
Good, more comments inline, more tests indicating how v0 and v1 will interact are needed. ...
4 years, 1 month ago (2016-11-10 03:43:09 UTC) #6
yurak
On 2016/11/10 at 03:43:09, dominicc wrote: > Good, more comments inline, more tests indicating how ...
4 years, 1 month ago (2016-11-10 10:53:26 UTC) #10
dominicc (has gone to gerrit)
Comments inline. https://codereview.chromium.org/2477713003/diff/120001/third_party/WebKit/LayoutTests/custom-elements/spec/create-element.html File third_party/WebKit/LayoutTests/custom-elements/spec/create-element.html (right): https://codereview.chromium.org/2477713003/diff/120001/third_party/WebKit/LayoutTests/custom-elements/spec/create-element.html#newcode18 third_party/WebKit/LayoutTests/custom-elements/spec/create-element.html:18: constructor() { There's no need to write ...
4 years, 1 month ago (2016-11-15 07:39:54 UTC) #11
yurak
On 2016/11/15 at 07:39:54, dominicc wrote: > Comments inline. > > https://codereview.chromium.org/2477713003/diff/120001/third_party/WebKit/LayoutTests/custom-elements/spec/create-element.html > File third_party/WebKit/LayoutTests/custom-elements/spec/create-element.html ...
4 years, 1 month ago (2016-11-16 02:55:56 UTC) #12
dominicc (has gone to gerrit)
More comments inline. https://codereview.chromium.org/2477713003/diff/160001/third_party/WebKit/Source/core/dom/Document.cpp File third_party/WebKit/Source/core/dom/Document.cpp (right): https://codereview.chromium.org/2477713003/diff/160001/third_party/WebKit/Source/core/dom/Document.cpp#newcode746 third_party/WebKit/Source/core/dom/Document.cpp:746: // 5. If 'is' is non-null ...
4 years, 1 month ago (2016-11-17 01:36:15 UTC) #13
yurak
On 2016/11/17 at 01:36:15, dominicc wrote: > More comments inline. > > https://codereview.chromium.org/2477713003/diff/160001/third_party/WebKit/Source/core/dom/Document.cpp > File ...
4 years, 1 month ago (2016-11-17 06:43:49 UTC) #14
dominicc (has gone to gerrit)
LGTM One comment inline about the TODO. https://codereview.chromium.org/2477713003/diff/180001/third_party/WebKit/Source/core/dom/Document.cpp File third_party/WebKit/Source/core/dom/Document.cpp (right): https://codereview.chromium.org/2477713003/diff/180001/third_party/WebKit/Source/core/dom/Document.cpp#newcode751 third_party/WebKit/Source/core/dom/Document.cpp:751: // TODO(yurak): ...
4 years, 1 month ago (2016-11-18 01:53:45 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2477713003/200001
4 years, 1 month ago (2016-11-18 03:21:28 UTC) #20
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_asan_rel_ng/builds/264951)
4 years, 1 month ago (2016-11-18 04:45:33 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2477713003/220001
4 years, 1 month ago (2016-11-18 05:48:07 UTC) #25
commit-bot: I haz the power
Committed patchset #12 (id:220001)
4 years, 1 month ago (2016-11-18 08:09:12 UTC) #26
commit-bot: I haz the power
4 years, 1 month ago (2016-11-18 08:12:05 UTC) #28
Message was sent while issue was closed.
Patchset 12 (id:??) landed as
https://crrev.com/fe200cd217b99368dbded68a0350d33c552c6a5b
Cr-Commit-Position: refs/heads/master@{#433146}

Powered by Google App Engine
This is Rietveld 408576698