1509060 - Port bug 1489833 - Replace use of nsIWindowMediator::getZOrderDOMWindowEnumerator

Status: RESOLVED FIXED

(Thunderbird :: General, task)

Description

[Jorg K (CEST = GMT+2)]

We have four call sites and one comment, suite/ has two more:

mail/base/content/mailCore.js
653 let windowList = Services.wm.getZOrderDOMWindowEnumerator("mail:3pane", true);

mail/base/content/msgMail3PaneWindow.js
670 // XXX We'd like to use getZOrderDOMWindowEnumerator here, but it doesn't work

mail/components/devtools/tb-root-actor.js
158 let winIter = Services.wm.getZOrderDOMWindowEnumerator(null, true);

mail/components/extensions/parent/ext-mail.js
132 let windowList = Services.wm.getZOrderDOMWindowEnumerator("mail:3pane", true);

mailnews/base/src/nsMsgMailSession.cpp
273 rv = windowMediator->GetZOrderDOMWindowEnumerator(nullptr, true, ...

For a replacement, see bug 1489833 comment #3, although I'm not sure what BrowserWindowTracker.getTopWindow is about.

Comment 1

[Dão Gottwald [:dao]]

(In reply to Jorg K (GMT+1) from comment #0)
> For a replacement, see bug 1489833 comment #3, although I'm not sure what
> BrowserWindowTracker.getTopWindow is about.

See https://searchfox.org/mozilla-central/search?q=BrowserWindowTracker.getTopWindow&case=true

Comment 2

[Jorg K (CEST = GMT+2)]

Thanks, we also have a C++ call site.

Comment 3

[Boris Zbarsky [:bzbarsky]]

For the C++ callsite you could fall back to the XP_UNIX path across the board for the moment, at least to keep things compiling.

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: 1509060-GetZOrderDOMWindowEnumerator.patch (v1)

This simple removes the Windows code paths which contain getZOrderDOMWindowEnumerator(). Looking at BrowserWindowTracker.jsm, it can't be used directly, instead as per bug 1489833 comment #3 we "should adopt something like BrowserWindowTracker.getTopWindow". I can't magically implement that when I have three hours at 9 PM.

Comment 5

[Jorg K (CEST = GMT+2)]

Attachment: 1509060-no-ws.patch

Here's a patch that ignores the indentation changes.

Boris, care to take a look? You have f? switched off. Try here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=11605bf53999ab4d5c223720ee24ac496fa1457e

Comment 6

[Jorg K (CEST = GMT+2)]

Attachment: 1509060-GetZOrderDOMWindowEnumerator.patch (v1b)

Remove a comment.

Comment 7

[Jorg K (CEST = GMT+2)]

Attachment: 1509060-no-ws.patch

Removed comment.

Comment 8

[Boris Zbarsky [:bzbarsky]]

> Boris, care to take a look? You have f? switched off

Right, because I don't have time to take a look right now and probably for the next few weeks...  In any case, I don't know this stuff well enough to review intelligently.

Comment 9

[Jorg K (CEST = GMT+2)]

Comment on attachment 9026800 [details] [diff] [review]
1509060-no-ws.patch

Geoff, what do you think of this? I'll have to land it when bug 1489833 merges, most likely within one hour from now.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/aa31b79e0b50
Port bug 1489833: Remove use of nsIWindowMediator::GetZOrderDOMWindowEnumerator. rs=bustage-fix

Comment 11

[Geoff Lankow (:darktrojan)]

Comment on attachment 9026800 [details] [diff] [review]
1509060-no-ws.patch

Review of attachment 9026800 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine to me.

Comment 12

[Jorg K (CEST = GMT+2)]

OK, leave it at that then.

Comment 13

[José M. Muñoz]

The fix here wasn't correct causing bug 1708073, see analysis in bug 1708073 comment #13. At least nsMsgMailSession::GetTopmostMsgWindow() is broken and doesn't find the top-most window. Surprising that in 2.5 years since Nov. 2018 no one noticed.