1648683 - right click menu too big on retina screen, tooltips cut off on non-retina screen

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P1)

Description

[Timothy Nikkel (:tnikkel)]

1. Goto https://www.w3schools.com/tags/tryit.asp?filename=tryhtml_select
2. "right click" (two finger tap) anywhere on the iframe on the right.

Pretty often (~25% of the time) I get a very large context menu.

Bisected to bug 1592416.

macOS 10.15.5, I'm running 2019 16" macbook pro with an external (non-retina) monitor connected. The issue only seems to happen on the built-in retina screen.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1592416

Comment 2

[Timothy Nikkel (:tnikkel)]

The regressing bug seems like it would also be a very strong candidate to explain why I very commonly get tooltips that are cut off on my external non-retina screen and it started happening in the right time frame.

Comment 3

[Haik Aftandilian [:haik]]

I haven't been able to reproduce this so far on a MacBook Pro (15-inch, 2018). We may want to backout bug 1592416 from Beta to avoid causing this regression on Release.

@tnikkel, could you confirm which version of Firefox you're running and provide a screen shot if possible?

Comment 4

[Timothy Nikkel (:tnikkel)]

The right click menu thing I bisected using mozregression, so it used nightlies. I first saw the issue while testing something else, also on nightly. I see the tooltip thing on beta, haven't bisected it. I forgot to upload the screenshot last night. Will do so now.

Comment 5

[Timothy Nikkel (:tnikkel)]

Attachment: Screen Shot 2020-06-26 at 1.48.04 AM.png

Comment 6

[Timothy Nikkel (:tnikkel)]

I can help debug and/or fix this issue as I am familiar with the areas of the code touched by bug 1592416, however I am a little busy right now.

Comment 7

[Haik Aftandilian [:haik]]

@Ryan, this regression appears to be caused by bug 1592416. The larger popup menu is probably less severe than the original problem and we're late in the Beta schedule so I don't think a backout makes sense here. Could you take a look and let me know if you agree?

Comment 8

[Stephen A Pohl [:spohl]]

(In reply to Timothy Nikkel (:tnikkel) from comment #2)

The regressing bug seems like it would also be a very strong candidate to explain why I very commonly get tooltips that are cut off on my external non-retina screen and it started happening in the right time frame.

Is this different from bug 1592206 comment 1?

Comment 9

[Timothy Nikkel (:tnikkel)]

(In reply to Stephen A Pohl [:spohl] from comment #8)

(In reply to Timothy Nikkel (:tnikkel) from comment #2)

The regressing bug seems like it would also be a very strong candidate to explain why I very commonly get tooltips that are cut off on my external non-retina screen and it started happening in the right time frame.

Is this different from bug 1592206 comment 1?

I believe so, yes, but I can't prove it because I don't have reliable steps to reproduce. I never saw truncated tooltips in practice in my regular browsing until about the past month (I use beta for my regular browsing and I think the truncated tooltips corresponded with central merging to beta). Obviously I can reproduce the bug in bug 1592206, but I only saw that because I was following the steps to reproduce in that bug.

Comment 10

[Timothy Nikkel (:tnikkel)]

I just found what seem to be 100% reliable steps to reproduce the tooltip problem: open a bugmail, hover over a bug number, repeat a couple times.

With those steps I confirm that bug 1592416 caused the tooltip issue for me. (I used mozregression to launch the relevant changesets from autoland to confirm).

So in summary it seems that bug 1592416 causes the context menu to be too big sometimes on a retina screen, and tooltips to be too small on a non-retina screen. I can file a separate bug for the tooltip thing if you think that would be better.

Comment 11

[Stephen A Pohl [:spohl]]

Thanks for narrowing this down! I'll let :haik decide how he'd like to track these issues.

Comment 12

[Julien Cristau [:jcristau]]

78.0 ships tomorrow, so it's too late for a backout now; I suppose I can keep this on the radar for a possible dot release.

Comment 13

[Haik Aftandilian [:haik]]

I'm now able to reproduce the large right click menu. In my case I had to set my external monitor to the highest scaling setting (i.e. towards "More Space") in the macOS display settings. No progress on the root cause yet.

Comment 15

[Haik Aftandilian [:haik]]

Update on this. No root cause yet, but for the case I'm hitting, I've found that after creating the popup NSWindow, at first its backingScaleFactor property is returned as 1.0. Later it becomes 2.0. I suspect this is because the window is not associated with the correct screen at first. When drawing the widget with the 1.0 value it is oversized, but with 2.0 the sizing is correct.

Comment 16

[Sören Hentzschel]

Can we track this for Firefox 79, please? This is a pretty serious visual regression. It takes several seconds (!) on my device for context menus to appear with the right dimensions, but not only context menus are affected, also the saved logins have totally wrong dimensions for several seconds. And to see this 30 times per day (at least!) is… not optimal. :)

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Fx79 is already in RC week and this isn't a new regression. I can track it for the release and as possible ride-along for an RC respin or dot release, but I'm not considering it to be a blocker either.

Comment 19

[Haik Aftandilian [:haik]]

In testing this, I've found that the problem appears to be after we create the NSWindow for a popup in nsCocoaWindow::CreateNativeWindow(), the backingScaleFactor value returned by [mWindow backingScaleFactor] is wrong. (I thought this might be because we create the window with the wrong frame position, but the values we use to create the window are always wrong and this doesn't make a difference.)

Before the popup is displayed, nsCocoaWindow::Move() is called which calls [mWindow setFrameTopLeftPoint] and after that the [mWindow backingScaleFactor] returns the correct value. We get the BackingScaleFactorChanged() callback, but we don't recalculate the size of the popup before showing the window.

Calling [mWindow setFrameTopLeftPoint] right after creating the window makes the correct scale factor be used and the popup displays at the right size.

In the BackingScaleFactorChanged() callback, what is the right way to trigger the popup dimensions being recalculated now that the scale factor has changed? @spohl or @mstange, do you know the right way to do that?

The fix I have works in my testing, but I think it would be better to use BackingScaleFactorChanged().

Comment 20

[Haik Aftandilian [:haik]]

Attachment: Bug 1648683 - right click menu too big on retina screen, tooltips cut off on non-retina screen

Hacky fix - not ready to land.

Workaround a problem that occurs with multiple screens where the NSWindow backing scale factor is incorrect (until [NSWindow setFrameTopLeftPoint] is called) causing some popups to display too large.

Comment 21

[Haik Aftandilian [:haik]]

Attachment: Bug 1648683 - right click menu too big on retina screen, tooltips cut off on non-retina screen

Handle backing scale factor changes that happen during initial popup window setup.

Comment 22

[Haik Aftandilian [:haik]]

Attachment: Bug 1648683 - right click menu too big on retina screen, tooltips cut off on non-retina screen

Handle backing scale factor changes that happen during initial popup window setup.

Comment 23

[Haik Aftandilian [:haik]]

The current attached patch is working well in the configurations I've tested. If anyone on the bug is able to test the changes on a multi-monitor setup and confirm that popups appear at the correct location at the correct size, that would help improve confidence in the fix.

A test build target.dmg can be downloaded from treeherder:

• Try push: https://treeherder.mozilla.org/#/jobs?repo=try&selectedTaskRun=M6cmHqORSJaNFCrQ6HYe1A.0&revision=2fb7c11cc2a65ae06c7c308208a5abbfca7a287e
• Build: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/JF3OrAKQS9mOzcVFGsb8Eg/runs/0/artifacts/public/build/target.dmg

Running the build downloaded from treeherder requires right-click open because it is not notarized. Alternatively, after dragging Nightly from the target.dmg disk image to the desktop, clear the quarantine attribute with $ xattr -c -r ~/Desktop/Firefox\ Nightly.app/ and then launch it normally.

@Timothy, could you test the attached patch or treeherder build and verify the problem no longer occurs?

Comment 24

[Timothy Nikkel (:tnikkel)]

I reproduce the bug in nightly first and then tested your build and I could not reproduce the bug in your try build. Thanks for fixing!

Comment 25

[Julien Cristau [:jcristau]]

We're reaching mid-beta 80, I'm going to call this fix-optional.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7940bff12885
right click menu too big on retina screen, tooltips cut off on non-retina screen r=spohl

Comment 27

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/7940bff12885

Comment 28

[Sören Hentzschel]

Thanks for working on it! Unfortunately I was not able to test earlier because I was on PTO. It is much better now but not completely fixed. While it works most times sometimes there is still a visible resizing for a short but noticeable moment. It's best to reproduce when you right click on a webpage, then on a link on the webpage (where the context menu has a different width, at least in a German build) and repeat this a few times if it doesn't happen the first time.

I hoped this would also fix the similar issue with the saved login fields but this issue is not fixed. That issue is even worse because it doesn't resize at all, it keeps the wrong size. I'll file a separate bug for this (edit: filed bug 1661439).