1720694 - WM_WINDOW_ROLE no longer set in Firefox 90

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Sergey Vlasov]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:90.0) Gecko/20100101 Firefox/90.0

Steps to reproduce:

After updating Firefox from 89.0.2 to 90.0 the WM_WINDOW_ROLE properties for various Firefox windows have changed, and the new values became less useful (in 90.0 all Firefox windows, including the "Page Info" window and the "About Firefox" window, have the same WM_WINDOW_ROLE value of "Toplevel").

Actual results:

In Firefox 90.0 the main browser window has these properties (as shown by "xprop WM_CLASS WM_WINDOW_ROLE"):
WM_CLASS(STRING) = "Browser", "Firefox"
WM_WINDOW_ROLE(STRING) = "Toplevel"

The Ctrl+I (Page Info) window:
WM_CLASS(STRING) = "Navigator", "Firefox"
WM_WINDOW_ROLE(STRING) = "Toplevel"

The Help/About Firefox window:
WM_CLASS(STRING) = "Navigator", "Firefox"
WM_WINDOW_ROLE(STRING) = "Toplevel"

So while the main browser window can still be distinguished by the different instance name in WM_CLASS, the Page Info and About windows are now indistinguishable without looking at the window title (which is localized and, in case of Page Info, not constant).

Expected results:

In Firefox 89.0.2 and older versions the WM_WINDOW_ROLE values were different and could be used to distinguish various windows.

The main browser window:
WM_CLASS(STRING) = "Navigator", "Firefox"
WM_WINDOW_ROLE(STRING) = "browser"

The Ctrl+I (Page Info) window:
WM_CLASS(STRING) = "Browser", "Firefox"
WM_WINDOW_ROLE(STRING) = "page-info"

The Help/About Firefox window:
WM_CLASS(STRING) = "Browser", "Firefox"
WM_WINDOW_ROLE(STRING) = "About"

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Sergey Vlasov]

Looks like this has been caused by https://bugzilla.mozilla.org/show_bug.cgi?id=1703073 — it removed mGtkWindowRoleName = role; from nsWindow::SetWindowClass(), therefore the window role that could be specified in the parameter is ignored, and the default "Toplevel" value is used. However, the motivation for that change looks strange — it's for Wayland purposes, but gdk_window_set_role() is effectively a noop on Wayland, and the only other place where mGtkWindowRoleName is used is some debugging output.

Comment 3

[Sergey Vlasov]

(In reply to Sergey Vlasov from comment #0)

In Firefox 90.0 the main browser window has these properties (as shown by "xprop WM_CLASS WM_WINDOW_ROLE"):

Sorry, I pasted wrong values when preparing the bug report. I checked the results of xprop WM_CLASS WM_WINDOW_ROLE on Firefox 90.0 again, and here are the values with correct WM_CLASS.

Main browser window:

WM_CLASS(STRING) = "Navigator", "Firefox"
WM_WINDOW_ROLE(STRING) = "Toplevel"

Page Info window:

WM_CLASS(STRING) = "Browser", "Firefox"
WM_WINDOW_ROLE(STRING) = "Toplevel"

“About Firefox“ window:

WM_CLASS(STRING) = "Browser", "Firefox"
WM_WINDOW_ROLE(STRING) = "Toplevel"

Developer Tools in a detached window:

WM_CLASS(STRING) = "Devtools", "Firefox"
WM_WINDOW_ROLE(STRING) = "Toplevel"

Bookmark manager (Ctrl+Shift+O):

WM_CLASS(STRING) = "Places", "Firefox"
WM_WINDOW_ROLE(STRING) = "Toplevel"

Although many of those windows still have distinct WM_CLASS values, some of them do not, and before the mentioned change they had distinct WM_WINDOW_ROLE values.

Comment 4

[Amir M. Saeid]

Attachment: Bug 1720694 - Set mGtkWindowRoleName so WM_WINDOW_ROLE can be set properly. r=jhorak

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0b00484e32d4
Set mGtkWindowRoleName so WM_WINDOW_ROLE can be set properly. r=stransky

Comment 6

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

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

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/0b00484e32d4

Comment 8

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

The patch landed in nightly and beta is affected.
:amirsaied, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 9

[Pascal Chevrel:pascalc]

Martin, is that something we should uplift to beta?

Comment 10

[Martin Stránský [:stransky] (ni? me)]

(In reply to Pascal Chevrel:pascalc from comment #9)

Martin, is that something we should uplift to beta?

It does not hurt us to uplift that.

Comment 11

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 9231399 [details]
Bug 1720694 - Set mGtkWindowRoleName so WM_WINDOW_ROLE can be set properly. r=jhorak

Beta/Release Uplift Approval Request

• User impact if declined: Desktop custom window handling setup does not work while it worked before.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We removed this code in Bug 1703073 but it seems to be needed for some custom window filtest so let's place it back. The patch sets various window names as was set before Bug 1703073.
• String changes made/needed: none

Comment 12

[Pascal Chevrel:pascalc]

Comment on attachment 9231399 [details]
Bug 1720694 - Set mGtkWindowRoleName so WM_WINDOW_ROLE can be set properly. r=jhorak

Low risk regression fix on Linux, approved for 91 beta 5, thanks.

Comment 13

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/bb2c8b4d3344