New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add <a> to safelist of elements that can call attachShadowRoot #511

Closed
PaulKinlan opened this Issue Jun 2, 2016 · 13 comments

Comments

Projects
None yet
6 participants
@PaulKinlan

PaulKinlan commented Jun 2, 2016

I noticed that anchors are not in the list of elements that can support attachShadowRoot and I was hoping that we could add them in. I didn't want to pollute #110

My current usage is a custom share button (demo) that I am creating (based on the fact that not all pages have a URL I am creating a component that allow the user to share the current URL).

I am currently allowing the developer to upgrade any element on the page to replace with my custom widget and I am looking to be as progressive as possible. If JS, or Web Components aren't supported I specifically want to keep the default link to a sharing service that I define (Twitter for example) — For example viewing the demo in Current Chrome 51 will just render a link, in Chrome Canary (52+ I think) I would have a custom widget.

Anchor rendering when no support for ShadowDOM:

No Support for WC, shows a link

If there is support for WC and Shadow DOM I would expect to be able to stamp in new structure and attach a new Shadow Root.

Safari Tech Preview currently allows me to attachShadowRoot to an anchor - the appears to be not spec compliant, but works as I want it to.

screen shot 2016-06-02 at 10 51 03 am

Chrome (Canary) being compliant to the whitelist, doesn't work as I want.

screen shot 2016-06-02 at 10 50 41 am

I understand that by default an anchor isn't inert and has a default action when the user clicks it and I am not sure what to do in that case, in my head I would expect to implicitly preventDefault and manage everything myself. Testing Safari TP, it looks like clicking the anchor will invoke the default action and as a developer I would need to override it.

edit: adding note about Safari TP, clicking on item does default action.

@hayatoito

This comment has been minimized.

Member

hayatoito commented Jun 6, 2016

Thank you for a feature request.

I understand the use case, however, as you noticed, this issue is not easy as we thought at first.

We have a history for that:

AFAIR, we stopped working on supporting these non-trivial elements because it took much time for us to define good consistent semantics on each element. We tried to support, however, we could not get a good implementation both in theory and in practical. Thus, we made it "WONTFIX". Yeah, we gave up, unfortunately... :(

I am not saying that it is impossible to support these elements, however, no one is trying to solve this issue, nowadays.

If we are to support this again, it would be nice to have a good consistent theory which can be applied consistently to any elements. Unless that, we might fail again.

I do not recall what issues existed exactly. However, I remember I had a lot of troubles in supporting Shadow DOM for <img> or <iframe>.
e.g. https://bugs.chromium.org/p/chromium/issues/detail?id=253801

@annevk

This comment has been minimized.

Member

annevk commented Jun 6, 2016

Standard: https://dom.spec.whatwg.org/#dom-element-attachshadow

I don't see a problem adding just a. It doesn't have special rendering rules.

@rniwa any reason Safari deviated from the standard? Can you tell us what Safari is shipping?

@rniwa

This comment has been minimized.

Contributor

rniwa commented Jun 6, 2016

We had implemented the old spec which disallowed attachShadow on a handful of elements. I'm fixing that in https://bugs.webkit.org/show_bug.cgi?id=157706 to match the latest spec now.

@annevk

This comment has been minimized.

Member

annevk commented Jun 6, 2016

Opinions on a? Seems like we should add that? Any others? First bit of developer feedback we're getting here on the new API.

@rniwa

This comment has been minimized.

Contributor

rniwa commented Jun 6, 2016

Given we don't want to support is= in custom elements, this would imply that authors have to manually attach shadow root on each anchor element. I'm not sure if that's useful enough to add the support. Also, what features of a should continue to function? e.g. should the activation behavior continue to work even when a shadow tree is attached?

Thinking of this problem further, we seem to have overlooked the mechanism by which to delegate or intercept the activation happening at the shadow host. Like focus, you may need to be able to intercept any activation on the host and overtake it. Similarly, there might be cases in which activating an element inside a shadow tree should activate the host instead.

@annevk

This comment has been minimized.

Member

annevk commented Jun 6, 2016

I was assuming that would not be any different from how nested a elements function today, but there might be something we overlooked. But yeah, that is a way in which a is somewhat special.

@rniwa

This comment has been minimized.

Contributor

rniwa commented Jun 6, 2016

I'm inclined to say we should hold off on this support until we figure out supporting other kinds of elements since that may lead to undesirable inconsistencies but I can be convinced otherwise if there is a strong enough reason to support it.

@PaulKinlan

This comment has been minimized.

PaulKinlan commented Jun 8, 2016

Thanks for the feedback. In my head I wanted the activation of a to work as it would without a shadowRoot and then I get to override it. For me even without a is=... the fact that I can encapsulate the logic in a custom class (not custom elements just yet) and effectively handle redistribution of nodes via slots was good enough for me to want to use JS to enhance the element, or leave it alone when either JS or ShadowDOM is not available.

It might be for another bug, but @annevk asked, I was expecting to see the following in the allowed list:

  • time (github used it in the past iirc, but now they have a custom element for displaying relative time)
  • ul, ol, li, dl, dd, dt
  • figure and figcaption
  • dialog (/me ducks and runs)

I also was thinking about embed and object as ways of encapsulating a flash->js converter, but that was pie in the sky for me.

@annevk

This comment has been minimized.

Member

annevk commented Jun 8, 2016

I think all those elements are probably okay. None of those would require a shadow root in user agents. But maybe it's best if we don't rock the agreement boat for now and add those once a couple browsers started shipping. There's a lot of other lose ends to tie up as well after all.

@prushforth

This comment has been minimized.

prushforth commented Aug 9, 2016

Hi, I made a comment over in #110, perhaps that was out of place - sorry for the noise. I am working on a customized built-in element in Polymer, and it is working quite nicely so far. I am a bit surprised at the neg on is=".. here. But mostly, I want to say that I want to provide enhanced behaviour for the <map> and <area> elements, which I guess are related/similar to <a> so I'm adding this comment here. My element demo can be found here: http://maps4html.github.io/Web-Map-Custom-Element/ or http://maps4html.github.io/Web-Map-Custom-Element/map-vs-web-map.html or a few other pages in that gh-pages branch.

If attachShadowRoot is not permitted for these elements, I am thinking that the idea of a customized built-in <map> element is not going to be possible, is that correct? That would be a shame as it takes away a great fallback behaviour that I am looking for - that of the existing <map> and associated <area> elements.

@annevk annevk referenced this issue Feb 18, 2018

Closed

Extend `attachShadow()` safelist #571

0 of 2 tasks complete
@annevk

This comment has been minimized.

Member

annevk commented Feb 18, 2018

Tracked upstream at whatwg/dom#571.

@TakayoshiKochi

This comment has been minimized.

Member

TakayoshiKochi commented Feb 20, 2018

@annevk let's close this and continue there?

@annevk

This comment has been minimized.

Member

annevk commented Feb 20, 2018

As long as we remember to dupe all future attachShadow() expansion requests to that issue, that seems fine.

@annevk annevk closed this Feb 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment