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