atlas-engineer / nyxt

Nyxt - the hacker's browser.
https://nyxt-browser.com/
9.64k stars 404 forks source link

Fix Shadow DOM performance issue and restore its support #3365

Closed heiwiper closed 2 months ago

heiwiper commented 3 months ago

Description

The algorithm for the macro rqsa has been improved by using Tree Walker instead of Node Iterator and the matching elements are now sorted in depth first order. The sorting order was necessary to fix the performance issue, You can read more about the details of that issue in the references issues below.

The algorithm for the macro rqs-nyxt-id has been improved by directly retrieving Shadow root elements if the matching element cannot be found by executing querySelect function. Previously, we would walk through all elements to check whether each element is a Shadow root. Please note that we can't use this same method in the rqsa macro because the that macro might be executed on elements that do not yet have nyxt attributes assigned which isn't the case for rqs-nyxt-id.

Fixes #3295

Checklist:

jmercouris commented 2 months ago

Thank you @heiwiper! I want you to know that this PR is not unnoticed!

heiwiper commented 2 months ago

Thanks for the followup! I totally understand, especially considering that the related issue is marked as low priority.

aadcg commented 2 months ago

@heiwiper I've tested this branch and I think the results are amazing!

I took the richest URL in terms of hints (~8k) and I can't notice any difference between this branch and master. That means that shadow DOM support adds no overhead, as intended.

And the feature is working too. I've tested it with https://developer.servicenow.com/dev.do.

In short, I think it's mostly done. Please mark it as ready and add a changelog entry. We'll integrate these changes in the next release in the 3 series.

Thanks!

heiwiper commented 2 months ago

I have just added the changelog entry and rebased into master.

Thanks for taking the time to test these changes!

aadcg commented 2 months ago

Thanks again @heiwiper.

heiwiper commented 2 months ago

My pleasure!

Feel free to tag me in related issues.