Skip to content
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

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

Open
jamessimone opened this issue Jun 12, 2020 · 12 comments
Labels
External: Lightning Web Security / Locker Categorizes issue or PR as related to LWS or Lightning Locker.

Comments

@jamessimone
Copy link

jamessimone commented Jun 12, 2020

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: WICG/webcomponents#358 and WICG/webcomponents#479

@pmdartus
Copy link
Member

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.

@jamessimone
Copy link
Author

@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?

@pmdartus
Copy link
Member

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

@ekashida
Copy link
Member

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

@pmdartus
Copy link
Member

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?

@manuel-jasso
Copy link

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()

@pmdartus
Copy link
Member

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.

@manuel-jasso
Copy link

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.

@caridy
Copy link
Contributor

caridy commented Jun 26, 2020

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.

@pmdartus
Copy link
Member

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.

@caridy
Copy link
Contributor

caridy commented Jun 30, 2020

@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.

@pmdartus pmdartus added the External: Lightning Web Security / Locker Categorizes issue or PR as related to LWS or Lightning Locker. label Jan 13, 2021
@lukethacoder
Copy link
Contributor

Is there a new workaround for getting the activeElement?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External: Lightning Web Security / Locker Categorizes issue or PR as related to LWS or Lightning Locker.
Projects
None yet
Development

No branches or pull requests

6 participants