1431429 - [GTK+ 3.10.8 / JWM] Titlebar drawing not eneabled when XDG_CURRENT_DESKTOP is missing

Status: ASSIGNED

(Core :: Widget: Gtk, defect, P5)

Description

[Terrell Kelley]

This was originally implemented in Bug 1414212 but was temporarily removed in order to fix a regression (Bug 1423810). 

The regression was fixed in Bug 1424974, so let's reenable for GTK+ >= 3.10

(Note: This was already reenabled for GTK 3.20+ in Bug 1417933.)

Comment 1

[Terrell Kelley]

Note: I have made the assumption that this was just not reenabled for GTK+ >= 3.10. It could possibly be something else. 

SPECS
Ubuntu 14.04 LTS
GTK+ version 3.10.8]
JWM Window Manager (https://joewing.net/projects/jwm/)

Comment 2

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

Can you please run latest trunk and check what settings do work for you? Set browser.tabs.drawInTitlebar to true at about:config and run:

$MOZ_GTK_TITLEBAR_DECORATION=client firefox
$MOZ_GTK_TITLEBAR_DECORATION=system firefox

the "client" should work at least.

Comment 3

[Terrell Kelley]

I will assume "latest trunk" means the latest Nightly. 

Both of those work (without the $). But ./firefox by itself does not.

Comment 4

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

Thanks, that looks good. Can you please also attach a screenshot of firefox at client and system mode running on some white background?

I'd like to see how the window borders and resize grips are rendered and if there are shadows round the main window. Also please attach a content of XDG_CURRENT_DESKTOP env variable, you can get it by:

env | grep XDG_CURRENT_DESKTOP

on terminal.

Thanks.

Comment 5

[Terrell Kelley]

Attachment: client-mode

Comment 6

[Terrell Kelley]

Attachment: system-mode

Comment 7

[Terrell Kelley]

Attachment: no options set

I have also included a version without either set, in case that is relevant for your second paragraph. As you can see, there are no shadows, and no indicators for the resizers (the cursor just changes). 

Unfortunately, it seems that JWM does not currently set $XDG_CURRENT_DESKTOP, nor any similar variables. I do note that `wmctrl -m` correctly detects the window manager as JWM. 

I also note that my previous bug 1424158 is still present, which is why I have a broken image on the close button, and no other window control buttons. I am using the Compact density setting.

Finally, I note that the background is slightly off white, as I used the new tab page as the background. I didn't notice until after I had already uploaded the first two images.

Comment 8

[Terrell Kelley]

Quick additional question: Would getting the borders help with fixing the fact that the border appears on top, even when maximized? Should I maybe also attach an image of what it looks like when maximized? 

Also, note that I included that about `wmctrl -m` to say that at least some apps can detect the WM. I'm hoping this one can be detected, too. Though, worst case, I'll just modify my start script to set that variable to JWM before launching Firefox.

Comment 9

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

(In reply to Terrell Kelley from comment #7)
> Created attachment 8944065 [details]
> no options set
> 
> I have also included a version without either set, in case that is relevant
> for your second paragraph. As you can see, there are no shadows, and no
> indicators for the resizers (the cursor just changes). 
> 
> Unfortunately, it seems that JWM does not currently set
> $XDG_CURRENT_DESKTOP, nor any similar variables. I do note that `wmctrl -m`
> correctly detects the window manager as JWM. 
> 
> I also note that my previous bug 1424158 is still present, which is why I
> have a broken image on the close button, and no other window control
> buttons. I am using the Compact density setting.
> 
> Finally, I note that the background is slightly off white, as I used the new
> tab page as the background. I didn't notice until after I had already
> uploaded the first two images.

This is a bug at JWM - it should set XDG_CURRENT_DESKTOP shomehow. We don't enable titlebar drawing when XDG_CURRENT_DESKTOP is not set right now.

Comment 10

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1552613

Comment 12

[Artem S. Tashkinov]

This has never been fixed.

Comment 13

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1431429 - Allow CSD even without XDG_CURRENT_DESKTOP. r=stransky

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b39ab94454e9
Allow CSD even without XDG_CURRENT_DESKTOP. r=stransky

Comment 16

[Sandor Molnar[:smolnar]]

Backed out changeset b39ab94454e9 (bug 1431429) for causing mochitest failures @ test_arrowpanel.html / browser_unified_extensions_overflowable_toolbar.js & dt failures @ browser_html_tooltip_screen_edge.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/1b56c653a5ee3a14ec0955b092498999e9333bfa

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_unified_extensions_overflowable_toolbar.js | Test timed out

Failure log -> TEST-UNEXPECTED-FAIL | toolkit/content/tests/chrome/test_arrowpanel.xhtml | Test timed out

Failure log -> TEST-UNEXPECTED-FAIL | devtools/client/shared/test/browser_html_tooltip_screen_edge.js | Test timed out

Comment 17

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1431429 - Fix some devtools test to correctly move the window to the right edge on Linux with CSD. r=#devtools-reviewers

This fixes the DevTools time-out that got me backed out below. In Linux
with CSD enabled, the allowable screen range for a window should include
some extra size (to account for the window decorations).

See some of the other users of these properties in the tree.

Before this bug, the relevant code is not exercised on automation.

Comment 18

[Emilio Cobos Álvarez [:emilio]]

Blegh, so my try run used artifact builds so that's why it didn't catch comment 16, fail on my side.

Martin, how worth it do you think this is? Comment 17 shows some of the kinds of fixes that are needed, but a lot of the existing failures aren't reproducible on my end (even on GNOME+X11), so it's probably not-low-effort to fix... If you think it's worth it I can take fixing all this stuff up over time, I guess...

Comment 19

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

(In reply to Emilio Cobos Álvarez (:emilio) from comment #18)

Blegh, so my try run used artifact builds so that's why it didn't catch comment 16, fail on my side.

Martin, how worth it do you think this is? Comment 17 shows some of the kinds of fixes that are needed, but a lot of the existing failures aren't reproducible on my end (even on GNOME+X11), so it's probably not-low-effort to fix... If you think it's worth it I can take fixing all this stuff up over time, I guess...

I think we can leave it after testsuite migration to 22.04 and set XDG_CURRENT_DESKTOP there so we test what we ship. I don't think it's worth to spend time on it right now.

Comment 20

[Dan Schomburg]

Attachment: WIP: Backed out changeset b39ab94454e9 (Bug 1431429) for causing mochitest failures @ test_arrowpanel.html / browser_unified_extensions_overflowable_toolbar.js CLOSED TREE

Comment 21

[Emilio Cobos Álvarez [:emilio]]

Wrong bug # / commit message?

Comment 22

[Dan Schomburg]

Yes sorry, I was experimenting with something, botched a rebase, and accidentally pushed to phabricator. I've abandoned https://phabricator.services.mozilla.com/D207175 ...