keepassxreboot / keepassxc-browser

KeePassXC Browser Extension
GNU General Public License v3.0
1.68k stars 172 forks source link

Notifications (errors) sometimes partly invisible when frame is too narrow (iframes, narrow window) #2255

Open Chealer opened 3 weeks ago

Chealer commented 3 weeks ago

KeePassXC error messages after trying to fill in credentials are not fully visible when their containing frame is less wide than ~520 pixels. This happens if the viewport has such a width, but also in some webpages―including at least on Pidgin's ITS, which is powered by JetBrains YouTrack and offers to login via a 333 pixels inline frame―regardless of the viewport's width.

Expected Behavior

The notification's full contents ("Error! Non connecté à KeePassXC.") should be visible: image

Current Behavior

The start of the notification's text is hidden, as can be seen in the following screenshot: KeePassXC notification

Possible Solution

The notification's width (520 pixels) exceeds that of the dialog supposed to contain it (397 px). This is a CSS bug caused by .kpxc-notification's width: 520px. Changing to max-width suffices to fix, but I can't say why even that would be needed. Removing the property altogether fixes for me, and even improves the result when this bug doesn't happen.

A more robust approach would be to avoid playing with the DOM and just display notifications outside of the tab's content.

Steps to Reproduce (for bugs)

  1. Make sure that KeePassXC is not open
  2. Open https://issues.imfreedom.org/projects
  3. Activate the Log in... button (top right)
  4. Activate KeePassXC's login form button (the icon in the form's first field)

Affected environments

KeePassXC 2.7.9 KeePassXC-Browser - 1.9.0.5 Operating system: Windows 11 Browser: Chrome, Firefox 127, Edge

Debugging tips

KeePassXC automatically hides its notifications after 5 s, making it hard to use the inspector for debugging. To workaround that without changing the extension's code, one may put a breakpoint on kpxcUI.createNotification's last line (closing brace) and run clearTimeout(notificationTimeout); in the console before continuing.

varjolintu commented 3 weeks ago

I have a fix in my TODO list that would send the notification to the main page if it's triggered from iframe. This looks exactly like that kind of scenario.

Chealer commented 3 weeks ago

Oops! The stack was so deep that I failed to notice that there's indeed an iframe halfway there, set to a width of 333 pixels. Thank you @varjolintu , I'll adjust the description accordingly.