Bug 188336 - HTML parser should execute custom element reactions for setting attributes immediately after creating a custom element
Summary: HTML parser should execute custom element reactions for setting attributes im...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks: 154907 183931 187319
  Show dependency treegraph
 
Reported: 2018-08-05 18:42 PDT by Ryosuke Niwa
Modified: 2018-08-13 19:55 PDT (History)
10 users (show)

See Also:


Attachments
Fixes the bug (4.20 KB, patch)
2018-08-05 18:59 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Added a missing change log (5.03 KB, patch)
2018-08-05 19:00 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Patch for landing (35.99 KB, patch)
2018-08-07 14:03 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Patch for landing (33.08 KB, patch)
2018-08-07 14:06 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Patch for landing (33.08 KB, patch)
2018-08-07 14:06 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2018-08-05 18:42:47 PDT
HTML parser should be pushing & popping from the custom element reactions stack:
https://html.spec.whatwg.org/multipage/parsing.html#create-an-element-for-the-token
Comment 1 Ryosuke Niwa 2018-08-05 18:59:47 PDT
Created attachment 346607 [details]
Fixes the bug
Comment 2 Ryosuke Niwa 2018-08-05 19:00:51 PDT
Created attachment 346608 [details]
Added a missing change log
Comment 3 Radar WebKit Bug Importer 2018-08-05 19:01:45 PDT
<rdar://problem/42954915>
Comment 4 Ryosuke Niwa 2018-08-05 19:10:49 PDT
Comment on attachment 346608 [details]
Added a missing change log

View in context: https://bugs.webkit.org/attachment.cgi?id=346608&action=review

> Source/WebCore/html/parser/HTMLDocumentParser.cpp:216
> +            m_treeBuilder->didCreateCustomOrFallbackElement(WTFMove(newElement), *constructionData);

Note that the insertion step doesn't need to be split because there is no way to affect the list of open elements in the parser once we forbid document.open/write in the bug 187319.
Comment 5 Frédéric Wang (:fredw) 2018-08-05 21:41:23 PDT
Comment on attachment 346608 [details]
Added a missing change log

View in context: https://bugs.webkit.org/attachment.cgi?id=346608&action=review

> Source/WebCore/ChangeLog:13
> +        is set or not since the precense of an element queue should not be observable in the case where we're constructing

nit: presence
Comment 6 Ryosuke Niwa 2018-08-06 00:51:15 PDT
Did you mean to r+ the patch with that comment?
Comment 7 Frédéric Wang (:fredw) 2018-08-06 00:57:05 PDT
(In reply to Ryosuke Niwa from comment #6)
> Did you mean to r+ the patch with that comment?

The patch looks good to me, but maybe someone else should review since I worked on this issue in bug 188190.
Comment 8 Ryosuke Niwa 2018-08-06 00:58:42 PDT
(In reply to Frédéric Wang (:fredw) from comment #7)
> (In reply to Ryosuke Niwa from comment #6)
> > Did you mean to r+ the patch with that comment?
> 
> The patch looks good to me, but maybe someone else should review since I
> worked on this issue in bug 188190.

Fair enough although this code change is quite bit different from what you made in that bug.
Comment 9 Frédéric Wang (:fredw) 2018-08-06 01:03:58 PDT
(In reply to Ryosuke Niwa from comment #8)
> (In reply to Frédéric Wang (:fredw) from comment #7)
> > (In reply to Ryosuke Niwa from comment #6)
> > > Did you mean to r+ the patch with that comment?
> > 
> > The patch looks good to me, but maybe someone else should review since I
> > worked on this issue in bug 188190.
> 
> Fair enough although this code change is quite bit different from what you
> made in that bug.

Right, let's take this then.
Comment 10 Ryosuke Niwa 2018-08-06 10:45:44 PDT
Committed r234608: <https://trac.webkit.org/changeset/234608>
Comment 11 Ryosuke Niwa 2018-08-07 14:02:58 PDT
Reopening to attach new patch.
Comment 12 Ryosuke Niwa 2018-08-07 14:03:00 PDT
Created attachment 346728 [details]
Patch for landing
Comment 13 Ryosuke Niwa 2018-08-07 14:03:39 PDT
Comment on attachment 346728 [details]
Patch for landing

Oops, wrong bug.
Comment 14 Ryosuke Niwa 2018-08-07 14:06:11 PDT
Created attachment 346730 [details]
Patch for landing
Comment 15 Ryosuke Niwa 2018-08-07 14:06:51 PDT
Created attachment 346731 [details]
Patch for landing
Comment 16 Ryosuke Niwa 2018-08-07 14:07:17 PDT
Comment on attachment 346731 [details]
Patch for landing

Ugh... wrong bug again.