browser.tabs.drawInTitlebar;true causes tabbar to have empty space to left and right

RESOLVED FIXED in Firefox 59

Status

()

defect
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: yoasif, Assigned: stransky)

Tracking

(Blocks 1 bug, {nightly-community, regression})

Trunk
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox57 unaffected, firefox58 unaffected, firefox59 fixed)

Details

Attachments

(8 attachments)

(Reporter)

Description

a year ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20171119100329

Steps to reproduce:

1. Start Firefox on Linux
2. Install https://addons.mozilla.org/en-us/firefox/addon/tab-counter-webext/ - which places an icon to the right of the tab bar by default. 


Actual results:

Empty space to the right of the icon. 


Expected results:

As when browser.tabs.drawInTitlebar is set to false.
(Reporter)

Comment 1

a year ago
(Reporter)

Comment 2

a year ago
 8:19.47 INFO: Narrowed inbound regression window from [a9cf5391, 63224d6e] (4 builds) to [9f8b177e, 63224d6e] (2 builds) (~1 steps left)
 8:19.47 INFO: No more inbound revisions, bisection finished.
 8:19.47 INFO: Last good revision: 9f8b177edf71215098643f5a705fe8730734c346
 8:19.47 INFO: First bad revision: 63224d6e543cc3399f419dd4ac500873db02b021
 8:19.47 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=9f8b177edf71215098643f5a705fe8730734c346&tochange=63224d6e543cc3399f419dd4ac500873db02b021
Blocks: 1415481
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → Widget: Gtk
Product: Firefox → Core
(Reporter)

Comment 3

a year ago
Also reported on  Comment 30 on bug 1415481.
(Assignee)

Comment 4

a year ago
I think we should remove "widget.allow-client-side-decoration" and use "browser.tabs.drawInTitlebar" only to disable/enable titlebar drawing.
(Assignee)

Updated

a year ago
Assignee: nobody → stransky
(Assignee)

Comment 5

a year ago
(In reply to Johann Hofmann [:johannh] from comment #31)
> Screenshots showing the added drag space:
> 
> https://screenshots.mattn.ca/compare/?oldProject=mozilla-
> central&oldRev=e070277ec199fa96fa490ed52d33646a376d0d80&newProject=mozilla-
> central&newRev=60b9fa15e4272bb1d2b876235f346587be6f2424&filter=linux
> 
> I guess that's intended to be shown by default.
(Assignee)

Updated

a year ago
Blocks: gtktitlebar
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8930074 - Flags: review?(jhorak) → review?(dao+bmo)

Comment 11

a year ago
mozreview-review
Comment on attachment 8930074 [details]
Bug 1418829 - set browser.tabs.drawInTitlebar to false on UNIX_BUT_NOT_MAC again,

https://reviewboard.mozilla.org/r/201254/#review206848
Attachment #8930074 - Flags: review?(dao+bmo) → review+
Duplicate of this bug: 1418263

Comment 13

a year ago
mozreview-review
Comment on attachment 8930075 [details]
Bug 1418829 - remove widget.allow-client-side-decoration preference,

https://reviewboard.mozilla.org/r/201256/#review206920
Attachment #8930075 - Flags: review?(jhorak) → review+

Comment 14

a year ago
mozreview-review
Comment on attachment 8930077 [details]
Bug 1418829 - nsLookAndFeel enables rendering to the titlebar for supported window managers only,

https://reviewboard.mozilla.org/r/201260/#review206356

::: widget/gtk/nsLookAndFeel.cpp:1081
(Diff revision 1)
>  
>      gtk_widget_destroy(window);
>      g_object_unref(labelWidget);
>  
> -    // Require GTK 3.10 for GtkHeaderBar support.
> -    mCSDAvailable = gtk_check_version(3, 10, 0) == nullptr;
> +    // Require GTK 3.10 for GtkHeaderBar support and compatible window manager.
> +    mCSDAvailable = (gtk_check_version(3, 10, 0) == nullptr &&

Remove the call of GetCSDSupportLevel() from 
https://searchfox.org/mozilla-central/source/widget/gtk/nsWindow.cpp#3758
and let this implementation of nsLookAndFeel::EnsureInit to decide whenever the CSD is available on not by checking GetCSDSupportLevel.

Consider the need of caching mIsCSDAvailable in nsWindow, is it worth? Calling: LookAndFeel::GetInt(LookAndFeel::eIntID_GTKCSDAvailable, &isCSDAvailable) looks cheap to me, or is the nsWindow::SetDrawsInTitlebar called so frequently?
Attachment #8930077 - Flags: review?(jhorak) → review-

Comment 15

a year ago
mozreview-review
Comment on attachment 8930076 [details]
Bug 1418829 - Export GetCSDSupportLevel() as public from nsWindow,

https://reviewboard.mozilla.org/r/201258/#review206340
Attachment #8930076 - Flags: review?(jhorak) → review+

Comment 16

a year ago
mozreview-review
Comment on attachment 8930078 [details]
Bug 1418829 - Add MATE entry to GetCSDSupportLevel(), use GDK_DECOR_BORDER with CSD_SUPPORT_FULL only,

https://reviewboard.mozilla.org/r/201262/#review206930
Attachment #8930078 - Flags: review?(jhorak) → review+
(Assignee)

Comment 17

a year ago
mozreview-review
Comment on attachment 8930077 [details]
Bug 1418829 - nsLookAndFeel enables rendering to the titlebar for supported window managers only,

https://reviewboard.mozilla.org/r/201260/#review206932

::: widget/gtk/nsLookAndFeel.cpp:1081
(Diff revision 1)
>  
>      gtk_widget_destroy(window);
>      g_object_unref(labelWidget);
>  
> -    // Require GTK 3.10 for GtkHeaderBar support.
> -    mCSDAvailable = gtk_check_version(3, 10, 0) == nullptr;
> +    // Require GTK 3.10 for GtkHeaderBar support and compatible window manager.
> +    mCSDAvailable = (gtk_check_version(3, 10, 0) == nullptr &&

I can't remove it from nsWindow class, it's going to be used for subsequent patches to get FULL/FLAT transparency type, set RGBA visual accordingly and fiddle with resizers.

So the code CSD at nsWindow::Create() is going to be extended for cases at Bug 1417933.
(Assignee)

Comment 18

a year ago
mozreview-review-reply
Comment on attachment 8930077 [details]
Bug 1418829 - nsLookAndFeel enables rendering to the titlebar for supported window managers only,

https://reviewboard.mozilla.org/r/201260/#review206932

> I can't remove it from nsWindow class, it's going to be used for subsequent patches to get FULL/FLAT transparency type, set RGBA visual accordingly and fiddle with resizers.
> 
> So the code CSD at nsWindow::Create() is going to be extended for cases at Bug 1417933.

Okay, I'll do that but I'd rather leave that for another patch as nsWindow::Create needs to be redesigned for FLAT/FULL modes and also round courners rendering.
(Assignee)

Updated

a year ago
Blocks: 1419448
(Assignee)

Comment 19

a year ago
mozreview-review
Comment on attachment 8930077 [details]
Bug 1418829 - nsLookAndFeel enables rendering to the titlebar for supported window managers only,

https://reviewboard.mozilla.org/r/201260/#review206938

::: widget/gtk/nsLookAndFeel.cpp:1081
(Diff revision 1)
>  
>      gtk_widget_destroy(window);
>      g_object_unref(labelWidget);
>  
> -    // Require GTK 3.10 for GtkHeaderBar support.
> -    mCSDAvailable = gtk_check_version(3, 10, 0) == nullptr;
> +    // Require GTK 3.10 for GtkHeaderBar support and compatible window manager.
> +    mCSDAvailable = (gtk_check_version(3, 10, 0) == nullptr &&

Let's track that at Bug 1419448.

Comment 20

a year ago
mozreview-review
Comment on attachment 8930077 [details]
Bug 1418829 - nsLookAndFeel enables rendering to the titlebar for supported window managers only,

https://reviewboard.mozilla.org/r/201260/#review206940

Okay, we'll address the nsWindow issues in bug 1419448. Please fix the typo in the commit message, r+ with that fixed.
Attachment #8930077 - Flags: review- → review+
Duplicate of this bug: 1419443
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Blocks: 1417933
(Assignee)

Updated

a year ago
Blocks: 1419456

Comment 27

a year ago
Pushed by stransky@redhat.com:
https://hg.mozilla.org/integration/autoland/rev/686eee770f7f
set browser.tabs.drawInTitlebar to false on UNIX_BUT_NOT_MAC again, r=dao
https://hg.mozilla.org/integration/autoland/rev/b757a8721ea2
remove widget.allow-client-side-decoration preference, r=jhorak
https://hg.mozilla.org/integration/autoland/rev/b055cf0f0b05
Export GetCSDSupportLevel() as public from nsWindow, r=jhorak
https://hg.mozilla.org/integration/autoland/rev/ff5a8099c8e3
nsLookAndFeel enables rendering to the titlebar for supported window managers only, r=jhorak
https://hg.mozilla.org/integration/autoland/rev/6918d1218831
Add MATE entry to GetCSDSupportLevel(), use GDK_DECOR_BORDER with CSD_SUPPORT_FULL only, r=jhorak
Martin, first, thanks for your great work to implement this also on Linux. On https://hg.mozilla.org/mozilla-central/rev/ff5a8099c8e3#l1.38 you are checking for "browser.tabs.drawInTitlebar". Thunderbird can now also draw in title bar. But we are using "mail.tabs.drawInTitlebar" to enable this feature. With your check, TB can't draw in title bar. Would it be possible to somehow change this check to work with TB too? Or do we need to change our pref to browser... which isn't very logical for a mail program?
Flags: needinfo?(stransky)
(Assignee)

Comment 30

a year ago
(In reply to Richard Marti (:Paenglab) from comment #29)
> Martin, first, thanks for your great work to implement this also on Linux.
> On https://hg.mozilla.org/mozilla-central/rev/ff5a8099c8e3#l1.38 you are
> checking for "browser.tabs.drawInTitlebar". Thunderbird can now also draw in
> title bar. But we are using "mail.tabs.drawInTitlebar" to enable this
> feature. With your check, TB can't draw in title bar. Would it be possible
> to somehow change this check to work with TB too? Or do we need to change
> our pref to browser... which isn't very logical for a mail program?

Good point, I'll investigate that. You may need to implement variant of Bug 1414222 which enables/disables titlebar according to system configuration and also provides info about active titlebar buttons. Then we can work out the "browser.tabs.drawInTitlebar" issue for TB.
Flags: needinfo?(stransky)
(Assignee)

Comment 31

a year ago
(In reply to Martin Stránský [:stransky] from comment #30)
> Good point, I'll investigate that. You may need to implement variant of Bug
> 1414222 which enables/disables titlebar according to system configuration
> and also provides info about active titlebar buttons. Then we can work out
> the "browser.tabs.drawInTitlebar" issue for TB.

Filed as Bug 1420110
(In reply to Martin Stránský [:stransky] from comment #31)
> (In reply to Martin Stránský [:stransky] from comment #30)
> > Good point, I'll investigate that. You may need to implement variant of Bug
> > 1414222 which enables/disables titlebar according to system configuration
> > and also provides info about active titlebar buttons. Then we can work out
> > the "browser.tabs.drawInTitlebar" issue for TB.

This is all already implemented in TB. :)

> Filed as Bug 1420110

Thanks.

Comment 34

a year ago
This is marked as fixed, but the problem still persists on my end:

Screenshots - https://imgur.com/a/FnYUt

Build-ID 	20171216100407
OS 	        Antergos (Gnome 3.26.2; GTK3 3.22.26)

Comment 35

a year ago
Like said in comment 34 this isn't fixed for us as well!

When the browser is maximized everything is ok, BUT if the browser is resized then suddenly there is an empty space left to the tabs. When maximizing the browser again the space disappears again!

Added a screenshot which shows that bug.

I am using GNOME-Flashback under Ubuntu 17.10.

This is my environment:

~$ env | grep "XDG_"
XDG_MENU_PREFIX=gnome-flashback-
XDG_GREETER_DATA_DIR=/var/lib/lightdm-data/mkurz
XDG_SESSION_TYPE=x11
XDG_DATA_DIRS=/usr/share/gnome-flashback-compiz:/usr/share/gnome-flashback-compiz:/usr/local/share:/usr/share:/var/lib/snapd/desktop:/var/lib/snapd/desktop
XDG_SESSION_DESKTOP=gnome-flashback-compiz
GNOME_SESSION_XDG_SESSION_PATH=/org/freedesktop/DisplayManager/Session0
XDG_SEAT_PATH=/org/freedesktop/DisplayManager/Seat0
XDG_CURRENT_DESKTOP=GNOME-Flashback:GNOME
XDG_RUNTIME_DIR=/run/user/1000
XDG_SESSION_PATH=/org/freedesktop/DisplayManager/Session0
XDG_CONFIG_DIRS=/etc/xdg/xdg-gnome-flashback-compiz:/etc/xdg/xdg-gnome-flashback-compiz:/etc/xdg
I've seen this on Linux/GNOME/Wayland. I can't reproduce it right now, but at some point it was triggered by switching a YouTube player to full-screen and back. Perhaps the 3ship and m.kurz can test this.

Comment 37

a year ago
@Laurentiu Nicola I already gave a description how to reproduce that. Just resize the window, that's it, nothing more.
In maximized state everything is OK, in un-maximized state the space on the left appears. Just testet with todays nightly.
@m.kurz Confirmed.

Comment 39

a year ago
(In reply to m.kurz from comment #35)
> Empty space on the left next to tabs when browser is resized

That's an intended part of the Photon design spec with titlebar disabled and unrelated to this bug.

https://mozilla.invisionapp.com/share/ZKBC94BPQ#/screens/237801528

Comment 40

a year ago
(In reply to Kestrel from comment #39)
> That's an intended part of the Photon design spec with titlebar disabled and
> unrelated to this bug.
> 
> https://mozilla.invisionapp.com/share/ZKBC94BPQ#/screens/237801528

Hmm.. Alright. I find it a bit weird because the UI seems "jumping" around when switching between maximized and un-maximized.

Comment 41

a year ago
(In reply to Kestrel from comment #39)
> (In reply to m.kurz from comment #35)
> > Empty space on the left next to tabs when browser is resized
> 
> That's an intended part of the Photon design spec with titlebar disabled and
> unrelated to this bug.
> 
> https://mozilla.invisionapp.com/share/ZKBC94BPQ#/screens/237801528

Is the empty space on the right of the tab bar supposed to be there, too?

As you can see from my screenshot in comment #34, when I drag an icon to the right of the tab bar, it leaves some space between that icon and the window control buttons, like described in comment #0. This empty space - unlike the one left of the tab bar - doesn't disappear, when I maximize the window.

Comment 42

a year ago
(In reply to 3ship from comment #41)
> Is the empty space on the right of the tab bar supposed to be there, too?

Yes, you can see these referred to as "Persistent Drag Spaces" in the design spec.

https://mozilla.invisionapp.com/share/ZKBC94BPQ#/screens/229786513
You need to log in before you can comment on or make changes to this bug.