Closed
Bug 1418829
Opened 7 years ago
Closed 7 years ago
browser.tabs.drawInTitlebar;true causes tabbar to have empty space to left and right
Categories
(Core :: Widget: Gtk, defect)
Core
Widget: Gtk
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox57 | --- | unaffected |
firefox58 | --- | unaffected |
firefox59 | --- | fixed |
People
(Reporter: yoasif, Assigned: stransky)
References
(Blocks 1 open bug)
Details
(Keywords: nightly-community, regression)
Attachments
(8 files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
text/x-review-board-request
|
dao
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jhorak
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jhorak
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jhorak
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jhorak
:
review+
|
Details |
(deleted),
image/png
|
Details |
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•7 years ago
|
||
Reporter | ||
Comment 2•7 years 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
status-firefox59:
--- → affected
Component: Untriaged → Widget: Gtk
Keywords: nightly-community,
regression
Product: Firefox → Core
Reporter | ||
Comment 3•7 years ago
|
||
Also reported on Comment 30 on bug 1415481.
Assignee | ||
Comment 4•7 years ago
|
||
I think we should remove "widget.allow-client-side-decoration" and use "browser.tabs.drawInTitlebar" only to disable/enable titlebar drawing.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → stransky
Assignee | ||
Comment 5•7 years 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•7 years ago
|
Blocks: gtktitlebar
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8930074 -
Flags: review?(jhorak) → review?(dao+bmo)
Comment 11•7 years 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+
Comment 13•7 years 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•7 years 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•7 years 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•7 years 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•7 years 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•7 years 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 | ||
Comment 19•7 years 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•7 years 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+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 27•7 years 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
Comment 28•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/686eee770f7f
https://hg.mozilla.org/mozilla-central/rev/b757a8721ea2
https://hg.mozilla.org/mozilla-central/rev/b055cf0f0b05
https://hg.mozilla.org/mozilla-central/rev/ff5a8099c8e3
https://hg.mozilla.org/mozilla-central/rev/6918d1218831
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment 29•7 years ago
|
||
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•7 years 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•7 years 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
Comment 32•7 years ago
|
||
(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 33•7 years ago
|
||
Updated•7 years ago
|
status-firefox57:
--- → unaffected
status-firefox58:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Comment 34•7 years 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•7 years 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
Comment 36•7 years ago
|
||
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•7 years 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.
Comment 38•7 years ago
|
||
@m.kurz Confirmed.
Comment 39•7 years 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•7 years 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•7 years 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•7 years 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.
Description
•