1402941 - Add HTMLSlotElement

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: slot_parser.diff

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: slot_dom.diff

The element isn't enabled yet

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

I just want to land the code to make it easier to land other Shadow DOM stuff

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

remote: View your change here:
remote:   https://hg.mozilla.org/try/rev/4282679f353799022b5476b3f6840878830edc34
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=4282679f353799022b5476b3f6840878830edc34

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: slot_dom.diff (fixed some nits)

Comment 6

[Henri Sivonen (:hsivonen)]

Comment on attachment 8912321 [details] [diff] [review]
slot_dom.diff (fixed some nits)

Review of attachment 8912321 [details] [diff] [review]:
-----------------------------------------------------------------

Not saying r+ quite yet pending answer to the question about AssignedNodeOptions getting ignored. (Also, please change the comment about clang-format.)

::: dom/html/HTMLSlotElement.cpp
@@ +52,5 @@
> +
> +void
> +HTMLSlotElement::AssignedNodes(const AssignedNodesOptions& aOptions,
> +                               nsTArray<RefPtr<nsINode>>& aNodes)
> +{

Why is aOptions ignored instead of having an effect per spec?

::: parser/html/java/README.txt
@@ +86,5 @@
>  # Save.
>  cd ../.. # Back to parser/html/java/
>  make translate
>  cd ../../..
> +# XXXsmaug clang-format doesn't deal well with some macros, so you may not want to use it.

Better to guide readers to exclude specific files from clang-format than to suggest not running it at all.

Comment 7

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #6)
> Better to guide readers to exclude specific files from clang-format than to
> suggest not running it at all.

Or sections of files, rather. With // clang-format off and // clang-format on

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

This is just adding the basic class. It is not doing anything yet, that is why 
HTMLSlotElement::AssignedNodes is very dummy.
Note, with this patch there isn't a way to create HTMLSlotElement yet, since the constructor call is explicitly commented out.

I can remove the XXXsmaug  comment, but it has been very painful to notice after wards that clang-format totally destroyed some files.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

s/destroyed/destroyed formatting of/

Comment 10

[Henri Sivonen (:hsivonen)]

Comment on attachment 8912321 [details] [diff] [review]
slot_dom.diff (fixed some nits)

Review of attachment 8912321 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Olli Pettay [:smaug] from comment #8)
> This is just adding the basic class. It is not doing anything yet, that is
> why 
> HTMLSlotElement::AssignedNodes is very dummy.
> Note, with this patch there isn't a way to create HTMLSlotElement yet, since
> the constructor call is explicitly commented out.

OK.

> I can remove the XXXsmaug  comment, but it has been very painful to notice
> after wards that clang-format totally destroyed some files.

I think it's good to warn readers to make a local backup commit before running ./mach clang-format, but I think it's not OK to suggest not running it at all.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Attachment: slot_dom_2.diff

Looks like I need to add the interface to the interface list on non-stylo, since we enable webcomponents in such cases in test profiles.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3704f4a298dc
Add HTMLSlotElement (disabled for now), r=hsivonen

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3704f4a298dc

Comment 14

[Chris Mills [:cmills]]

Documented:

https://developer.mozilla.org/en-US/docs/Web/API/HTMLSlotElement