lwc icon indicating copy to clipboard operation
lwc copied to clipboard

this.template.activeElement is not set when focusing markup injected by slot

Open jamessimone opened this issue 4 years ago • 12 comments

Description

Slot markup follows special rules in relation to the Shadow DOM:

DOM elements that are passed to a component via slots aren't owned by the component and aren't in the component's shadow tree. To access DOM elements passed via slots, call this.querySelector(). The component doesn't own these elements, so you don't use template.

Unfortunately, there is no corresponding tracking for focused elements when the focused elements are passed in by slot; there is no this.activeElement, for example, which only returns focused elements that are part of slot-passed-in markup.

Steps to Reproduce

I've included a sandboxed repro below:

https://developer.salesforce.com/docs/component-library/tools/playground/WXAr7bdAW/12/edit

This is the specific bit that ends up being problematic:

//focusableElems is a list where the first element is injected by slot
if (focusableElems.length > 0) {
    focusableElems[0].focus();
    console.log(this.template.activeElement) // output: null
}

If, for example, the snippet is updated to use focusableElems[1].focus(), then this.template.activeElement is immediately set.

Expected Results

Either this.template.activeElement tracks all markup, independent of whether it is part of the component's Shadow DOM, or an alternative means of tracking focus for injected markup is made available.

Actual Results

this.template.activeElement returns null when markup injected via slot is focused, and there is no alternative means to track selected elements.

Version

  • LWC: 1.6.0

Possible Solution

I'd like to hear thoughts on also exposing an activeElement property on the component itself, which will act just like the template version of activeElement while maintaining the existing separation of concerns that has already been built into distinguishing between component-owned markup versus what's being passed in.

Related to: https://github.com/w3c/webcomponents/issues/358 and https://github.com/w3c/webcomponents/issues/479

jamessimone avatar Jun 12 '20 15:06 jamessimone

The fact that this.template.activeElement return the activeElement in the shadow tree matches the specification. In order to access the active element in the light DOM you can use: this.template.host.getRootNode().activeElement. Then you would need to compare the activeElement position with the host element position in the DOM to know if the active element is slotted into the considered component or not. Here is your playground updated: https://developer.salesforce.com/docs/component-library/tools/playground/WXAr7bdAW/13/edit

IMO, we should stick to the standard material here and not add APIs activeElement on the LightningElement class if it doesn't have an equivalent in standard HTMLElement. That being said, I think exposing the getRootNode method would make more sense on the LightningElement to avoid having to look it up with the template.host.

pmdartus avatar Jun 16 '20 14:06 pmdartus

@pmdartus thanks for the detailed response - that works for me. I wasn't even aware of the host property on the template, previously; I'm sure it's part of the spec, but I'm not an expert on all aspects of that. I appreciate you filling in the blanks for me and updating the sandbox. Would you like for me to update the title/details of this issue to request exposing the getRootNode method?

jamessimone avatar Jun 16 '20 17:06 jamessimone

Before changing the title I would like to get the rest of the team opinion on this. /cc @caridy @ekashida.

pmdartus avatar Jun 17 '20 09:06 pmdartus

I think it's fine to expose getRootNode() on the LightningElement.

ekashida avatar Jun 22 '20 22:06 ekashida

Since this method would give access to the owner document or shadow root, this might have some implications from Locker's perspective. @manuel-jasso @caridy what do you think about this?

pmdartus avatar Jun 23 '20 08:06 pmdartus

The rules for Locker blocking access to this.template.host.* properties is different in current production Locker and vNext.

In current prod Locker the rules vary by namespace (I can check details if necessary), so the fix you're proposing might work.

But in vNext currently we're planning to disallow access to all of the this.template.host.* properties, so you wouldn't have access to this.template.host.getRootNode()

manuel-jasso avatar Jun 24 '20 21:06 manuel-jasso

If we were to block access to the root node, what would be our stance regarding this specific issue @manuel-jasso? While it's not common, there are still valid cases for accessing the root node.

pmdartus avatar Jun 25 '20 07:06 pmdartus

You make a valid point @pmdartus, real world use cases like this are helpful for us to shape Locker vNext. Our goal is "secure by default" which means we want to prevent accidental security anti-patterns, but we know there are valid use cases and maybe we need to have some configs available to allow certain things, but make it very explicit and intentional. I will make sure the @salesforce/ui-security team keeps this use case in mind as we continue with our design.

manuel-jasso avatar Jun 25 '20 18:06 manuel-jasso

this.template.host ~== this that approximation is problematic for locker, including locker vnext. So, thanks @pmdartus for raising this issue. getRootNode on the this will be very hard to control in vnext, therefore we should not add it yet, unless we find a better way to distort it. As today, because vnext doesn't have access to Lightning Element base class, it will not be able to distort any of its accessors, that's something to think about.

I believe this.template.host.getRootNode() workaround is good enough if you're not using locker, eventually, host getter will be smart enough to return the right value depending on the ownership, while today it is just not accessible for anybody.

caridy avatar Jun 26 '20 18:06 caridy

Do you think we should tap on the newly introduced Renderer for Locker to intercept some of the properties' access and methods invocations in VNext? Instead of the prototype patching approach used in current version of Locker.

pmdartus avatar Jun 29 '20 08:06 pmdartus

@pmdartus the idea for vnext is to not have to deal with that, but the problem is not about how to touch the dom, but how you interact with the component instance, which locker vnext doesn't do much today.

caridy avatar Jun 30 '20 18:06 caridy

Is there a new workaround for getting the activeElement?

lukethacoder avatar Nov 03 '21 01:11 lukethacoder