Support shadow dom #485
Support shadow dom #485
Conversation
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
| if (innerActiveElement?.shadowRoot) { | ||
| return getActiveElement(innerActiveElement.shadowRoot) | ||
| } |
There was a problem hiding this comment.
activeElement doesn't show through the boundary, so if there's a shadowRoot, it means that the real activeElement is inside the nested tree. Recourse.
There was a problem hiding this comment.
JS is not great with recursion + large call stacks, I wonder if this is a situation for a max_count or something like you've done before?. Although, if this is only added in shadow trees, perhaps it never sees deep DOM trees? not a blocker, just came to mind since JS lacks any kind of tail-call optimization :)
There was a problem hiding this comment.
Yeah, I thought about that. A previous implementation with elementsFromPoint did have something similar, but I figured, this only traverses nested shadow trees looking for a an activeElement. It doesn't call querySelectorAll or anything expensive. On the vast majority of cases, this will only recourse once or twice. To make this a significant problem, someone would have to nest shadow trees hundreds of times deep, one inside the other. That doesn't seem plausible. You usually have treeWithStuff > subTreeWithMoreStuff > field. Even if the subtrees are large, we don't care, because we only read the activeElement.
| // Add the shadow DOM listener. Handlers in handleEvent | ||
| window.addEventListener('pointerdown', this, true) | ||
| // We don't listen for focus events on mobile, they can cause keyboard flashing | ||
| if (!this.device.globalConfig.isMobileApp) { | ||
| window.addEventListener('focus', this, true) | ||
| } |
| this.findEligibleInputs(realTarget.getRootNode()) | ||
| } | ||
|
|
||
| window.performance?.mark?.('scan_shadow:init:end') |
There was a problem hiding this comment.
great to see more of this :)
src/autofill-utils.js
Outdated
| * Returns the event target, or an element that matches wantedTargetType, piercing the shadow tree | ||
| * @param {PointerEvent | FocusEvent} event | ||
| * @param {typeof Element} [wantedTargetType] | ||
| * @returns {*|Element} |
There was a problem hiding this comment.
I think this is like any|Element, it would be good to tighten that up (probably your editor made this). not a blocker at all
| if (innerActiveElement?.shadowRoot) { | ||
| return getActiveElement(innerActiveElement.shadowRoot) | ||
| } |
There was a problem hiding this comment.
JS is not great with recursion + large call stacks, I wonder if this is a situation for a max_count or something like you've done before?. Although, if this is only added in shadow trees, perhaps it never sees deep DOM trees? not a blocker, just came to mind since JS lacks any kind of tail-call optimization :)
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
… ema/support-shadow-dom Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> # Conflicts: # dist/autofill-debug.js # dist/autofill.js # swift-package/Resources/assets/autofill-debug.js # swift-package/Resources/assets/autofill.js
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Task/Issue URL: https://app.asana.com/0/1206475855296108/1206475855296108 Autofill Release: https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/10.1.0 ## Description Updates Autofill to version [10.1.0](https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/10.1.0). ### Autofill 10.1.0 release notes ## What's Changed * Support shadow dom by @GioSensation in duckduckgo/duckduckgo-autofill#485 * New batch of fixes + integration test improvements by @GioSensation in duckduckgo/duckduckgo-autofill#489 * Update password-related json files (2024-01-17) by @daxmobile in duckduckgo/duckduckgo-autofill#474 * Bump playwright from 1.38.1 to 1.40.1 by @dependabot in duckduckgo/duckduckgo-autofill#458 * Update password-related json files (2024-01-26) by @daxmobile in duckduckgo/duckduckgo-autofill#484 * Bump json-schema-to-typescript from 13.1.1 to 13.1.2 by @dependabot in duckduckgo/duckduckgo-autofill#479 **Full Changelog**: duckduckgo/duckduckgo-autofill@10.0.3...10.1.0 ## Steps to test This release has been tested during autofill development. For smoke test steps see [this task](https://app.asana.com/0/1198964220583541/1200583647142330/f). Co-authored-by: GioSensation <GioSensation@users.noreply.github.com>
Reviewer: @shakyShane
Asana: https://app.asana.com/0/0/1206427720288392/f
Description
Scans fields in shadow trees on click.
Steps to test
QA done during ship review. I will be adding integration tests as a fast-follow, but there's already a test page and I'm thinking of finally bringing in privacy test pages as a dependency for the integration tests. So many of these pages are duplicated.