1949890 - Message header popups open outside the application

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect, P1)

Description

[Alessandro Castellani [:aleca]]

STR:

• Open a single message in the message pane (it doesn't matter if it's the about3Pane, a tab, or a standalone window)
• Click on one of the recipients in the message header.

The menupopup opens at the bottom right of the monitor, outside the the application.

Comment 1

[Wayne Mery (:wsmwk)]

I've been seeing this for several days. It goes back to at least 2025-02-14 build

Calum, can you find the regression range?

Comment 2

[Alessandro Castellani [:aleca]]

I found out that event.screenX is actually returning the wrong value as it reports a bigger number than the correct value of the clicked event.
https://searchfox.org/comm-central/rev/7a274fcf6967456777f822a1f1f38d108a5ddb69/mail/base/content/msgHdrView.js#3005

Comment 3

[Martin Giger [:freaktechnik]]

I'm only seeing it on hi-dpi screens.

Comment 4

[Alessandro Castellani [:aleca]]

Emilio, are you aware of any m-c changes that might affect the screen size detection in HiDPI?
So weird that it only happens in the message header and nowhere else

Comment 5

[Martin Giger [:freaktechnik]]

3:29.54 INFO: Last good revision: 7f6d0270a648c0332ffec5a4b64060ff3210ddd0
3:29.54 INFO: First bad revision: 1d991e4853051fdbbb490ef6eaacd52a2e91929a
3:29.54 INFO: Pushlog:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=7f6d0270a648c0332ffec5a4b64060ff3210ddd0&tochange=1d991e4853051fdbbb490ef6eaacd52a2e91929a

Comment 6

[Martin Giger [:freaktechnik]]

Backing out bug 1931692 seems to fix it for me locally.

Comment 7

[Martin Giger [:freaktechnik]]

Setting flags based on regressor.

Comment 8

[Alessandro Castellani [:aleca]]

Gentle ping to Masayuki for some help on this.

Comment 9

[Martin Giger [:freaktechnik]]

The regression is gone even by just swapping these two lines back: https://searchfox.org/mozilla-central/rev/83e29f5ee2e301ac7224e2927bddda16634b1897/dom/events/UIEvent.cpp#167-171

Comment 10

[Emilio Cobos Álvarez [:emilio]]

Yeah so it seems this is only an issue if you query the event post dispatch, so a workaround could be getting it before the await in that function.

But it still seems there's a Gecko bug here. Masayuki can hopefully take a closer look. Thanks for finding it!

Comment 11

[Martin Giger [:freaktechnik]]

Attachment: Bug 1949890 - Work around screen coordinates being wrong on HiDPI screens after await. r=#thunderbird-front-end-reviewers

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm. Here is the original code.

void UIEvent::DuplicatePrivateData() {
  mLayerPoint = GetLayerPoint();

  // GetScreenPoint converts mEvent->mRefPoint to right coordinates.
  // Note that mPresContext will be cleared by Event::DuplicatePrivateData().
  // Therefore, we need to use mPresContext before calling it.
  const CSSIntPoint screenPoint = RoundedToInt(
      Event::GetScreenCoords(mPresContext, mEvent, mEvent->mRefPoint)
          .valueOr(CSSIntPoint{0, 0}));
  const CSSToLayoutDeviceScale scale = mPresContext
                                           ? mPresContext->CSSToDevPixelScale()
                                           : CSSToLayoutDeviceScale(1);

  Event::DuplicatePrivateData();
  MOZ_ASSERT_IF(!mEventIsInternal, !mPresContext);

  mEvent->mRefPoint = RoundedToInt(screenPoint * scale);
}

scale is computed before Event::DuplicatePrivateData() clears mPresContext. Then, it's used when caching mRefPoint which is now screen point rather than relative offset in the widget. This meaning change must cause making me have misunderstood...

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e8bd76d274b2
Work around screen coordinates being wrong on HiDPI screens after await. r=aleca

Comment 14

[Martin Giger [:freaktechnik]]

Comment on attachment 9467809 [details]
Bug 1949890 - Work around screen coordinates being wrong on HiDPI screens after await. r=#thunderbird-front-end-reviewers

[User impact if declined]

• When clicking on email adresses in the message header users with HiDPI screens will get the popup at the wrong screen position. It will likely be off screen.

[Is this code covered by automated tests?]

• No (I think partially, but not the regressing issue for sure)

[Has the fix been verified in Daily? (or Beta for an ESR uplift?)]

• It's been verified in builds off c-c. The fix is also well understood.

[Needs manual test from QA?]

• Unsure, I would tend toward yes?
• Have HiDPI screen
• Select message in 3pane
• Click on sender or recipient email
• Observe popup location

[List of other uplifts needed]

• Not aware of any dependencies

[Risk to taking this patch]

• Low

[Why is the change risky/not risky? (and alternatives if risky)]

• It only changes when we read values to work around the fairly well understood bug.

[String changes made/needed]

• none

Comment 15

[Corey Bryant]

Comment on attachment 9467809 [details]
Bug 1949890 - Work around screen coordinates being wrong on HiDPI screens after await. r=#thunderbird-front-end-reviewers

[Triage Comment]
Approved for beta

Comment 16

[Corey Bryant]

Thunderbird 136.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/6ab2d22ef311