Closed Bug 1733620 Opened 3 years ago Closed 3 years ago

[wayland] Form field popups are too small

Categories

(Core :: Widget: Gtk, defect, P2)

Desktop
Linux
defect

Tracking

()

RESOLVED FIXED
95 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox-esr91 --- unaffected
firefox92 --- unaffected
firefox93 --- unaffected
firefox94 + fixed
firefox95 + fixed

People

(Reporter: heftig, Assigned: stransky)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(8 files)

The warning popup that appears in password fields sent to an insecure website, like a router login form, is often too small or even invisible. (It seems it's still possible to click the invisible popup.)

I've seen similar things happening to form history popups.

GNOME Shell 40.5, 3840x2160 @ 200%, no scale-monitor-framebuffer, Arch Linux.

Last good revision: acf8d94482470230ccaa4ee2dac63bc335d77d7c
First bad revision: 101c8562810919310177d25f7795db52dafc3fb9
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=acf8d94482470230ccaa4ee2dac63bc335d77d7c&tochange=101c8562810919310177d25f7795db52dafc3fb9

Regressed by bug 1729709.

Blocks: wayland-popup

Can you please run with MOZ_LOG="WaylandPopup:5" and attach the log here?
Thanks.

Flags: needinfo?(jan.steffens)

This may be a dupe of Bug 1733754. Please run with WAYLAND_DEBUG=1 and attach the log here.
Thanks.

Can you make a try build for https://phabricator.services.mozilla.com/D127425 ? I'll see if your workaround fixed it.

Here's the screencast for the above log, with the log also visible. I'm mostly just clicking the password field.

https://pkgbuild.com/~heftig/Screencast%20from%2004-10-21%2019%3A30%3A30.webm

Flags: needinfo?(jan.steffens)
Attachment #9244232 - Attachment description: moz.log → Log with MOZ_LOG=WidgetPopup:5 WAYLAND_DEBUG=1

For the first look it seems to be different than Bug 1733754 but please try the try. I expect the page isn't available for testing, right?

(In reply to Martin Stránský [:stransky] (ni? me) from comment #6)

There's the try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d424735a14c3a4350bb1f6c4ef0af7b665b0a9b

Unfortunately, this bug still reproduces.


(In reply to Martin Stránský [:stransky] (ni? me) from comment #7)

For the first look it seems to be different than Bug 1733754 but please try the try. I expect the page isn't available for testing, right?

I found a publicly accessible login form that shows the popup when you enter the password field: http://testphp.vulnweb.com/login.php

Priority: -- → P2
Assignee: nobody → stransky

Right now we resize/move popup windows even when move-to-rect callback is pending which leads to wrong popup
position / size if popup is moved/resized repeatedly.

In this patch we do:

  • Remove multiple move/resize internal routines (NativeMove(), NativeResize()) and use only one method NativeMoveResize() for both.
  • Always use NativeMoveResizeWaylandPopup() for popup window positioning and check if we're waiting to move-to-rect callback.
  • When we're waiting to the callback save both new size and position (previously we saved size only) and use mBounds coordinates for it.
  • Remove position/size params from NativeMoveResize() / NativeMoveResizeWaylandPopup() and use mBounds internally and mBounds is already
    set and it's correct popup size/position.
  • Remove extra DispatchResized() call - it's already called from NativeMoveResize().
Pushed by stransky@redhat.com:
https://hg.mozilla.org/integration/autoland/rev/1c581d243b87
[Linux] Don't call move/resize when we're waiting to move-to-rect callback, r=jhorak
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch

This isn't fixed yet. I'm running https://hg.mozilla.org/mozilla-central/rev/3cf700ab977a298e19a00f1ec66d14939766955b and the popup still has drawing problems (see the first click) and sometimes only displays a sliver (later clicks). It doesn't seem to disappear entirely anymore, though.

Screencast for this log: https://pkgbuild.com/~heftig/Screencast%20from%2008-10-21%2013%3A30%3A08.webm

PS: The try build above behaves the same.

(whoops, duplicate comment)

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Regressions: 1734758
Regressions: 1734980

I'm unable to reproduce on Fedora 34. Jan, can you please attach log with MOZ_LOG="WaylandPopup:5" only?
Thanks.

Flags: needinfo?(jan.steffens)
Flags: needinfo?(jan.steffens)

(In reply to Martin Stránský [:stransky] (ni? me) from comment #15)

I'm unable to reproduce on Fedora 34. Jan, can you please attach log with MOZ_LOG="WaylandPopup:5" only?

What does "I'm unable to reproduce on Fedora 34" mean in terms of your GNOME Shell version, scaling and scale-monitor-framebuffer?

If there's a relevant patch to Mutter/Shell in Fedora that Arch is missing, I could add it.

Flags: needinfo?(stransky)
Regressions: 1735151

I'm using latest nightly, mutter-40.5-2.fc34.x86_64, gtk3-3.24.30-1.1.fc34.x86_64.

I think I understand what's going on. The popup is hidden while we're waiting for move-to-rect callback. When popup is shown again, we get the move-to-rect callback but we think that the popup size is the same so we take shortcut and does not resize it again. But Gtk created the popup small (minimal height is set to 4 pixels) so it stays small.

Flags: needinfo?(stransky)

Jan, can you please check this try if it fixes it for you?
https://treeherder.mozilla.org/#/jobs?repo=try&revision=867e4c6369aaa00d0c28bbb30f1287e75a2fdacc
Thanks.

Flags: needinfo?(jan.steffens)

Sorry, still the same behavior.

Flags: needinfo?(jan.steffens)

Hm, interesting, looks like Gdk reports wrong popup sizes to us.

Which mutter/gtk version do you run?

Please test this try, I removed the window size check:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=61c46f31518db4c17246ac19dd087d93fb3152fd

Also it's weird that move-to-rect callback is called after window hide/show.
Thanks.

Flags: needinfo?(jan.steffens)

It's Mutter 40.5 and GTK 3.24.30-62-g8d04980f38.

I double checked GTK and this issue disappears with the 3.24.30 release. I'll bisect this.

9a4e32892896ce1d0a92f413845f6f7f18f9b456 ("Ignore wl_output globals not bound by us") is the first bad commit.

This is from https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3941, which apparently was explicitly added for Firefox.

The try did not build (error: unused variable 'needsSizeUpdate').

Flags: needinfo?(jan.steffens)

It must be something else, https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3941 is not included in 3.24.30 (AFAIK).

I see that move-to-rect callback is not deleted when popup window is hidden but it's fired when it's shown again - is there any related changeset you're missing?

(In reply to Martin Stránský [:stransky] (ni? me) from comment #28)

It must be something else, https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3941 is not included in 3.24.30 (AFAIK).

Yes, and as I said, 3.24.30 does not exhibit the issue, but 3.24.30-62-g8d04980f38 does. I bisected between them.

Okay, let's close it then.

Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Regressions: 1735292
Blocks: 1735583

The patch landed in nightly and beta is affected.
:stransky, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.
If yes, don't forget to request an uplift for the patches in the regressions caused by this fix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(stransky)

The patch to too big and risky for backport.

Flags: needinfo?(stransky)
Regressions: 1736601

This seems important as it makes the password manager very hard to use on Wayland. If the fix won’t be backported, would it make sense to revert bug 1729709 for the 94 branch?

(In reply to Anders Kaseorg from comment #34)

This seems important as it makes the password manager very hard to use on Wayland. If the fix won’t be backported, would it make sense to revert bug 1729709 for the 94 branch?

I hit this bug on the provided testcase page only - it's not a general issues with all password manager popups. Or can you reproduce it everywhere?

Flags: needinfo?(andersk)

I was able to reproduce this (and am able to reproduce bug 1735583) with any kind of form popup: "insecure form" popups, login autofill, autocompletion (form history). Drop-down (<select>) popups seem to be unaffected.

Yeah, I see this on basically every website where I have a username+password saved. The dropdown listing saved usernames that’s supposed to toggle every time you click the username box, typically appears once truncated and then disappears forever. I had to switch from beta to nightly just for this bug.

https://fill.dev/form/login-simple makes for a good test case.

Flags: needinfo?(andersk)

Okay, let's revert bug 1729709 for FF94 then.
Thanks for the info!

[Tracking Requested - why for this release]: backout of the regressing bug requested for 94 rc2

Has Regression Range: --- → yes
Regressions: 1746812
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: