Skip to content

Ignore non-'visible' overlays#2894

Open
indivisible wants to merge 1 commit intokeepassxreboot:developfrom
indivisible:fix/check_overlay_visibility
Open

Ignore non-'visible' overlays#2894
indivisible wants to merge 1 commit intokeepassxreboot:developfrom
indivisible:fix/check_overlay_visibility

Conversation

@indivisible
Copy link
Copy Markdown

Check if popover / overlay is hidden with the visibility CSS property before checking if it blocks an input field.

Testing strategy

The bug was noticed on https://wizzair.com/ as the "Sign in" option's fields were often not found by the extension. Debugging showed that the fields were overlapping with the #navigation-language-selector language selection dialog, which had visibility: hidden in CSS. The bug might not appear on the site with different screen sizes. it's somewhat layout dependent

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)

@varjolintu varjolintu self-requested a review March 6, 2026 12:59
@varjolintu varjolintu added the bug label Mar 6, 2026
continue;
}

if (getComputedStyle(overlay).visibility != 'visible') {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use strict comparison !==.

@varjolintu
Copy link
Copy Markdown
Member

While testing this a little bit, the only time I got the input fields not to be detected is to resize the page to a small window, then maximize it again. Reloading the page always loads the input fields.

I'm a little worried that this kind of check might actually break the overlay protection we have added to the extension. When dealing with overlays, the proper way to override the checks is to use Custom Login Fields.

@indivisible
Copy link
Copy Markdown
Author

I tried that, but the page has a different bug with that: clicking an extension UI element closes the login dialog. Activating choose custom login fields and then clicking username will close the login dialog. Maybe the extension could prevent event bubbling, but if the page uses blur events for this then I don't really know how to reliably stop it. But that's another issue

As for messing up the overlay protection: after some testing it could cause problems that non-visible parents can have visible children. For such cases a proper check would be walking the whole tree and checking every node for visibility. But the overlay protection is already messed up anyway as this page demonstrates

The visibility checking code is janky, and can't really see any way to make it perfect in every situation. I think a useful hack would be adding a per-site setting to ignore or greatly neuter checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants