Skip to content

Conversation

@carsteneu
Copy link

While analyzing issue #13469 and #12972 (menu shrinking unexpectedly), I noticed a subtle
calculation issue in PopupResizeHandler._on_actor_show() that could contribute
to sizing problems under certain conditions.

The previous code compared the outer actor size against the work area but then
adjusted the inner content size. This works correctly most of the time, but can
lead to incorrect results when there's padding between the actor and its content
(e.g., from theme CSS) and timing issues occur during layout.

This fix explicitly calculates the padding offset between the outer actor and
inner content, then uses it to determine the correct maximum content size that
fits within the work area.

This is admittedly an edge case, but it becomes more relevant with UI scaling.
For example, on a 1366×768 laptop with 150% scaling and the menu resized to
maximum (900px logical = 1350px physical + ~30px padding = 1380px), the menu
actor exceeds the work area (1366px), triggering this code path with potentially
incorrect calculations.

Changes

  • Calculate padding between outer actor and inner content
  • Use padding-aware calculation for work area constraints
  • Ensures mathematically correct sizing regardless of layout timing

Tested

  • Tested in VM with Linux Mint 22.3 / Cinnamon 6.6.5
  • Menu opens/closes correctly
  • No JavaScript errors in logs
  • Multiple open/close cycles work as expected

Related

The previous calculation in _on_actor_show() compared the outer actor
size against the work area but then adjusted the inner content size,
which could lead to incorrect results when there's padding between
the actor and its content (e.g., from theme CSS).

This fix explicitly calculates the padding offset and uses it to
determine the correct maximum content size that fits within the
work area.
@fredcw
Copy link
Contributor

fredcw commented Jan 24, 2026

This is the same calculation except for the two Max() conditions. The first one Math.max(0, cur_w - user_w) only takes effect if the user size is greater than both the menu size and the workarea size, in which case it sets the new user size to the workarea size, which is wrong. The second Max() Math.max(0, this._workAreaHeight - padding_h) prevents the user size from going negative but that makes no difference because the applet obviously has to set a minimum size anyway.

You're obviously putting a lot of work into finding and fixing bugs and many or most of your PRs may very well be good and correct fixes so I certainly wouldn't want to say anything to discourage your efforts. But in this particular case, this PR and explanation don't make much sense and I suspect that's because it's almost 100% LLM generated. One thing's for sure, LLMs are only going to get better. In two years time, I doubt any of us will barely ever write a single line of code again. :o

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants