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.

Sign up for GitHub

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

Jump to bottom

:autofill pseudo class always returns true #129

Open
zuzusik opened this issue Oct 15, 2024 · 3 comments
Open

:autofill pseudo class always returns true #129

zuzusik opened this issue Oct 15, 2024 · 3 comments

Comments

@zuzusik
Copy link

zuzusik commented Oct 15, 2024 •
edited
Loading

It might be reasonable to always return false instead - autofill is not really possible in this environment

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Document</title>
    <script src="https://cdn.jsdelivr.net/npm/[email protected]/src/nwsapi.min.js" onload="NW.Dom.install"></script>
</head>
<body>
<div id="test">test</div>
<script>
    const element = NW.Dom.byId("test", document)[0];
    const isAutofill = NW.Dom.match(":autofill", element);
    document.write("isAutofill: " + isAutofill);
</script>
</body>
</html>
Screenshot 2024-10-09 at 2 29 43 PM

Autofill pseudo class: https://developer.mozilla.org/en-US/docs/Web/CSS/:autofill
@zuzusik zuzusik mentioned this issue Oct 15, 2024
Open
@dperini
Copy link
Owner

dperini commented Jan 3, 2025

@zuzusik
I am trying to reconstruct the history of these additions to the list of pseudo-class selector considered in nwsapi.

If I recall correctly the additions of these pseudo-class stems from a jsdom request to avoid breaking the process for all of the pseudo-class starting with "-webkit-" to be ignored and considered as invalid selectors, producing a "false" match but continuing the processing of the complete selector input. This was back in the days were none of these pseudo-classes was contemplated by any selectors specification.

Only later, when the "-webkit-autofill" started appearing as a possible pseudo-class selector developers realized that choice to be wrong and started to be a problem. At that point I tried to revert the code to return a "noop" instead of a "false" match result and ended up adding the lines after the existing ones while probably they should have been added before the single ":" and double "::" resolvers. Here is the relevant code lines in the sources:

        // allow pseudo-elements starting with single colon (:)
        // :after, :before, :first-letter, :first-line
        // assert: e.type is in double-colon format, like ::after
        else if ((match = selector.match(Patterns.pseudo_sng))) {
          source = 'if(e.element&&e.type.toLowerCase()=="' +
            ':' + match[0].toLowerCase() + '"){e=e.element;' + source + '}';
        }

        // allow pseudo-elements starting with double colon (::)
        // ::after, ::before, ::marker, ::placeholder, ::inactive-selection, ::selection, ::-webkit-<foo-bar>
        // assert: e.type is in double-colon format, like ::after
        else if ((match = selector.match(Patterns.pseudo_dbl))) {
          source = 'if(e.element&&e.type.toLowerCase()=="' +
            match[0].toLowerCase() + '"){e=e.element;' + source + '}';
        }

        // placeholder for parsed only no-op selectors
        else if ((match = selector.match(Patterns.pseudo_nop))) {
          break;
        }

        else {  
          
          // reset
          expr = false;
          status = false;

So currently that's the mess we are at. My suggestion is to change the resolver in a way it doesn't hurt us in the future when more of these sophisticated selectors will appear as valid. I am open to suggestions keeping in mind that we should absolutely mimic today's browsers functionality regarding this and if necessary have flags to let users change behaviour.

@dperini
Copy link
Owner

dperini commented Jan 3, 2025

Obviously the code should check if the property yielding the true result does effectively allow for such a property and pseudo-class as we normally do for any other selectors considered valid.

@zuzusik
Copy link
Author

zuzusik commented Jan 3, 2025

At that point I tried to revert the code to return a "noop" instead of a "false" match result and ended up adding the lines after the existing ones while probably they should have been added before the single ":" and double "::" resolvers

Yeah, this seems reasonable to me - looking at the snippet you provided I agree that Patterns.pseudo_nop should go before two other conditions, otherwise :autofill will be processed by the first one (Patterns.pseudo_sng)

we should absolutely mimic today's browsers functionality regarding this

I don't think it is really possible to detect auto-fill from Javascript, other than checking if the element is returned with respective .querySelector result, so to me it makes sense to mark :autofill as never present in NWSAPI, till there will be a good way to detect auto-filled elements from JS without tapping into selector engine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dperini @zuzusik