Skip to content

Commit

Permalink
Bug 1736518 - Make browser.tabs.drawInTitlebar a tri-state. r=stransk…
Browse files Browse the repository at this point in the history
…y,Gijs

To more properly support Linux having a different default at runtime.

Expose the resolved value in appinfo for convenience, and use it in the
front-end as needed.

Differential Revision: https://phabricator.services.mozilla.com/D129004
  • Loading branch information
emilio committed Oct 21, 2021
1 parent 127a09c commit f317384
Show file tree
Hide file tree
Showing 26 changed files with 112 additions and 104 deletions.
5 changes: 0 additions & 5 deletions browser/app/profile/firefox.js
Original file line number Diff line number Diff line change
Expand Up @@ -629,11 +629,6 @@ pref("browser.tabs.tabMinWidth", 76);
// secondary text on tabs hidden due to size constraints and readability
// of the text at small font sizes.
pref("browser.tabs.secondaryTextUnsupportedLocales", "ar,bn,bo,ckb,fa,gu,he,hi,ja,km,kn,ko,lo,mr,my,ne,pa,si,ta,te,th,ur,zh");
// Initial titlebar state is managed by -moz-gtk-csd-hide-titlebar-by-default
// on Linux.
#ifndef UNIX_BUT_NOT_MAC
pref("browser.tabs.drawInTitlebar", true);
#endif

//Control the visibility of Tab Manager Menu.
pref("browser.tabs.tabmanager.enabled", false);
Expand Down
5 changes: 1 addition & 4 deletions browser/base/content/browser-tabsintitlebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,7 @@ var TabsInTitlebar = {
_prefName: "browser.tabs.drawInTitlebar",

_readPref() {
let hiddenTitlebar = Services.prefs.getBoolPref(
"browser.tabs.drawInTitlebar",
window.matchMedia("(-moz-gtk-csd-hide-titlebar-by-default)").matches
);
let hiddenTitlebar = Services.appinfo.drawInTitlebar;
this.allowedBy("pref", hiddenTitlebar);
},

Expand Down
6 changes: 3 additions & 3 deletions browser/base/content/test/popups/browser_popupUI.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ add_task(async function titlebar_buttons_visibility() {
Services.prefs.setIntPref("browser.link.open_newwindow", 2);

const drawInTitlebarValues = [
[true, BUTTONS_MAY_VISIBLE],
[false, BUTTONS_NEVER_VISIBLE],
[1, BUTTONS_MAY_VISIBLE],
[0, BUTTONS_NEVER_VISIBLE],
];
const windowFeaturesValues = [
// Opens a popup
Expand All @@ -77,7 +77,7 @@ add_task(async function titlebar_buttons_visibility() {
const menuBarShownValues = [true, false];

for (const [drawInTitlebar, drawInTitlebarButtons] of drawInTitlebarValues) {
Services.prefs.setBoolPref("browser.tabs.drawInTitlebar", drawInTitlebar);
Services.prefs.setIntPref("browser.tabs.drawInTitlebar", drawInTitlebar);

for (const [
windowFeatures,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ add_task(async function testWritesEnabledOnPrefChange() {
is(enabled, 0, "Pre-XUL skeleton UI is disabled in the Windows registry");

Services.prefs.setBoolPref("browser.startup.preXulSkeletonUI", true);
Services.prefs.setBoolPref("browser.tabs.drawInTitlebar", false);
Services.prefs.setIntPref("browser.tabs.drawInTitlebar", 0);
enabled = WindowsRegistry.readRegKey(
Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
"Software\\Mozilla\\Firefox\\PreXULSkeletonUISettings",
Expand Down
5 changes: 1 addition & 4 deletions browser/components/BrowserGlue.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -800,10 +800,7 @@ let JSWINDOWACTORS = {
);

// Hide the titlebar if the actual browser window will draw in it.
let hiddenTitlebar = Services.prefs.getBoolPref(
"browser.tabs.drawInTitlebar",
win.matchMedia("(-moz-gtk-csd-hide-titlebar-by-default)").matches
);
let hiddenTitlebar = Services.appinfo.drawInTitlebar;
if (hiddenTitlebar) {
win.windowUtils.setChromeMargin(0, 2, 2, 2);
}
Expand Down
10 changes: 3 additions & 7 deletions browser/components/customizableui/CustomizableUI.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -3151,12 +3151,8 @@ var CustomizableUIInternal = {

_resetUIState() {
try {
// kPrefDrawInTitlebar may not be defined on Linux/Gtk+ which throws an exception
// and leads to whole test failure. Let's set a fallback default value to avoid that,
// both titlebar states are tested anyway and it's not important which state is tested first.
gUIStateBeforeReset.drawInTitlebar = Services.prefs.getBoolPref(
kPrefDrawInTitlebar,
false
gUIStateBeforeReset.drawInTitlebar = Services.prefs.getIntPref(
kPrefDrawInTitlebar
);
gUIStateBeforeReset.uiCustomizationState = Services.prefs.getCharPref(
kPrefCustomizationState
Expand Down Expand Up @@ -3246,7 +3242,7 @@ var CustomizableUIInternal = {
this._clearPreviousUIState();

Services.prefs.setCharPref(kPrefCustomizationState, uiCustomizationState);
Services.prefs.setBoolPref(kPrefDrawInTitlebar, drawInTitlebar);
Services.prefs.setIntPref(kPrefDrawInTitlebar, drawInTitlebar);
Services.prefs.setIntPref(kPrefUIDensity, uiDensity);
Services.prefs.setBoolPref(kPrefAutoTouchMode, autoTouchMode);
Services.prefs.setBoolPref(
Expand Down
7 changes: 2 additions & 5 deletions browser/components/customizableui/CustomizeMode.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -1715,10 +1715,7 @@ CustomizeMode.prototype = {
},

_updateTitlebarCheckbox() {
let drawInTitlebar = Services.prefs.getBoolPref(
kDrawInTitlebarPref,
this.window.matchMedia("(-moz-gtk-csd-hide-titlebar-by-default)").matches
);
let drawInTitlebar = Services.appinfo.drawInTitlebar;
let checkbox = this.$("customization-titlebar-visibility-checkbox");
// Drawing in the titlebar means 'hiding' the titlebar.
// We use the attribute rather than a property because if we're not in
Expand All @@ -1732,7 +1729,7 @@ CustomizeMode.prototype = {

toggleTitlebar(aShouldShowTitlebar) {
// Drawing in the titlebar means not showing the titlebar, hence the negation:
Services.prefs.setBoolPref(kDrawInTitlebarPref, !aShouldShowTitlebar);
Services.prefs.setIntPref(kDrawInTitlebarPref, !aShouldShowTitlebar);
},

_getBoundsWithoutFlushing(element) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,16 @@ add_task(async function() {

// Bug 971626 - Restore Defaults should collapse the Title Bar
add_task(async function() {
if (Services.appinfo.OS != "WINNT" && Services.appinfo.OS != "Darwin") {
return;
{
const supported = TabsInTitlebar.systemSupported;
is(typeof supported, "boolean");
info("TabsInTitlebar support: " + supported);
if (!supported) {
return;
}
}
let prefName = "browser.tabs.drawInTitlebar";
let defaultValue = Services.prefs.getBoolPref(prefName);

const kDefaultValue = Services.appinfo.drawInTitlebar;
let restoreDefaultsButton = document.getElementById(
"customization-reset-button"
);
Expand All @@ -166,7 +171,7 @@ add_task(async function() {
);
is(
titlebarCheckbox.hasAttribute("checked"),
!defaultValue,
!kDefaultValue,
"Title bar checkbox should reflect pref value"
);
is(
Expand All @@ -175,14 +180,20 @@ add_task(async function() {
"Undo reset button should be hidden at start of test"
);

Services.prefs.setBoolPref(prefName, !defaultValue);
let prefName = "browser.tabs.drawInTitlebar";
Services.prefs.setIntPref(prefName, !kDefaultValue);
ok(
!restoreDefaultsButton.disabled,
"Restore defaults button should be enabled when pref changed"
);
is(
Services.appinfo.drawInTitlebar,
!kDefaultValue,
"Title bar checkbox should reflect changed pref value"
);
is(
titlebarCheckbox.hasAttribute("checked"),
defaultValue,
kDefaultValue,
"Title bar checkbox should reflect changed pref value"
);
ok(
Expand All @@ -202,14 +213,19 @@ add_task(async function() {
);
is(
titlebarCheckbox.hasAttribute("checked"),
!defaultValue,
!kDefaultValue,
"Title bar checkbox should reflect default value after reset"
);
is(
Services.prefs.getBoolPref(prefName),
defaultValue,
Services.prefs.getIntPref(prefName),
2,
"Reset should reset drawInTitlebar"
);
is(
Services.appinfo.drawInTitlebar,
kDefaultValue,
"Default state should be restored"
);
ok(CustomizableUI.inDefaultState, "In default state after titlebar reset");
is(
undoResetButton.hidden,
Expand All @@ -228,13 +244,13 @@ add_task(async function() {
);
is(
titlebarCheckbox.hasAttribute("checked"),
defaultValue,
kDefaultValue,
"Title bar checkbox should reflect undo-reset value"
);
ok(!CustomizableUI.inDefaultState, "No longer in default state after undo");
is(
Services.prefs.getBoolPref(prefName),
!defaultValue,
Services.prefs.getIntPref(prefName),
kDefaultValue ? 0 : 1,
"Undo-reset goes back to previous pref value"
);
is(
Expand Down
11 changes: 2 additions & 9 deletions browser/modules/BrowserUsageTelemetry.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -497,9 +497,7 @@ let BrowserUsageTelemetry = {
case "browser.tabs.drawInTitlebar":
this._recordWidgetChange(
"titlebar",
Services.prefs.getBoolPref("browser.tabs.drawInTitlebar")
? "off"
: "on",
Services.appinfo.drawInTitlebar ? "off" : "on",
"pref"
);
break;
Expand Down Expand Up @@ -601,12 +599,7 @@ let BrowserUsageTelemetry = {
widgetMap.set("menu-toolbar", menuBarHidden ? "off" : "on");

// Drawing in the titlebar means not showing the titlebar, hence the negation.
widgetMap.set(
"titlebar",
Services.prefs.getBoolPref("browser.tabs.drawInTitlebar", true)
? "off"
: "on"
);
widgetMap.set("titlebar", Services.appinfo.drawInTitlebar ? "off" : "on");

for (let area of CustomizableUI.areas) {
if (!(area in BROWSER_UI_CONTAINER_IDS)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ function organizeToolbars(state = {}) {
targetState.personalToolbarVisible
);

Services.prefs.setBoolPref(
Services.prefs.setIntPref(
"browser.tabs.drawInTitlebar",
!targetState.titlebarVisible
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ var TabsInTitlebar = {
tabsInTitlebar: {
selectors: ["#navigator-toolbox"],
async applyConfig() {
if (Services.appinfo.OS == "Linux") {
return "TabsInTitlebar isn't supported on Linux";
}
Services.prefs.setBoolPref(PREF_TABS_IN_TITLEBAR, true);
Services.prefs.setIntPref(PREF_TABS_IN_TITLEBAR, 1);
return undefined;
},
},
Expand All @@ -30,7 +27,7 @@ var TabsInTitlebar = {
Services.appinfo.OS == "Linux" ? [] : ["#titlebar"]
),
async applyConfig() {
Services.prefs.setBoolPref(PREF_TABS_IN_TITLEBAR, false);
Services.prefs.setIntPref(PREF_TABS_IN_TITLEBAR, 0);
},
},
},
Expand Down
1 change: 0 additions & 1 deletion layout/style/test/chrome/bug418986-2.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ var suppressed_toggles = [
"-moz-windows-default-theme",
"-moz-windows-glass",
"-moz-gtk-csd-available",
"-moz-gtk-csd-hide-titlebar-by-default",
"-moz-gtk-csd-minimize-button",
"-moz-gtk-csd-maximize-button",
"-moz-gtk-csd-close-button",
Expand Down
1 change: 0 additions & 1 deletion layout/style/test/chrome/chrome-only-media-queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const CHROME_ONLY_TOGGLES = [
"-moz-windows-non-native-menus",
"-moz-swipe-animation-enabled",
"-moz-gtk-csd-available",
"-moz-gtk-csd-hide-titlebar-by-default",
"-moz-gtk-csd-minimize-button",
"-moz-gtk-csd-maximize-button",
"-moz-gtk-csd-close-button",
Expand Down
8 changes: 8 additions & 0 deletions modules/libpref/init/StaticPrefList.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1300,6 +1300,14 @@
value: false
mirror: always

# Whether we should draw the tabs on top of the titlebar.
#
# no (0), yes (1), or default (2), which is true everywhere except Linux.
- name: browser.tabs.drawInTitlebar
type: int32_t
value: 2
mirror: always

# If set, use DocumentChannel to directly initiate loads entirely
# from parent-process BrowsingContexts
- name: browser.tabs.documentchannel.parent-controlled
Expand Down
3 changes: 1 addition & 2 deletions servo/components/style/gecko/media_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ macro_rules! bool_pref_feature {
/// to support new types in these entries and (2) ensuring that either
/// nsPresContext::MediaFeatureValuesChanged is called when the value that
/// would be returned by the evaluator function could change.
pub static MEDIA_FEATURES: [MediaFeatureDescription; 59] = [
pub static MEDIA_FEATURES: [MediaFeatureDescription; 58] = [
feature!(
atom!("width"),
AllowsRanges::Yes,
Expand Down Expand Up @@ -890,7 +890,6 @@ pub static MEDIA_FEATURES: [MediaFeatureDescription; 59] = [
lnf_int_feature!(atom!("-moz-windows-glass"), WindowsGlass),
lnf_int_feature!(atom!("-moz-swipe-animation-enabled"), SwipeAnimationEnabled),
lnf_int_feature!(atom!("-moz-gtk-csd-available"), GTKCSDAvailable),
lnf_int_feature!(atom!("-moz-gtk-csd-hide-titlebar-by-default"), GTKCSDHideTitlebarByDefault),
lnf_int_feature!(atom!("-moz-gtk-csd-minimize-button"), GTKCSDMinimizeButton),
lnf_int_feature!(atom!("-moz-gtk-csd-maximize-button"), GTKCSDMaximizeButton),
lnf_int_feature!(atom!("-moz-gtk-csd-close-button"), GTKCSDCloseButton),
Expand Down
15 changes: 11 additions & 4 deletions toolkit/xre/nsAppRunner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "mozilla/Printf.h"
#include "mozilla/ResultExtensions.h"
#include "mozilla/ScopeExit.h"
#include "mozilla/StaticPrefs_browser.h"
#include "mozilla/StaticPrefs_fission.h"
#include "mozilla/Telemetry.h"
#include "mozilla/Utf8.h"
Expand Down Expand Up @@ -276,7 +277,6 @@ static const char kPrefThemeId[] = "extensions.activeThemeID";
static const char kPrefBrowserStartupBlankWindow[] =
"browser.startup.blankWindow";
static const char kPrefPreXulSkeletonUI[] = "browser.startup.preXulSkeletonUI";
static const char kPrefDrawTabsInTitlebar[] = "browser.tabs.drawInTitlebar";
#endif // defined(XP_WIN)

int gArgc;
Expand Down Expand Up @@ -1331,6 +1331,12 @@ nsXULAppInfo::GetChromeColorSchemeIsDark(bool* aResult) {
return NS_OK;
}

NS_IMETHODIMP
nsXULAppInfo::GetDrawInTitlebar(bool* aResult) {
*aResult = LookAndFeel::DrawInTitlebar();
return NS_OK;
}

NS_IMETHODIMP
nsXULAppInfo::GetProcessStartupShortcut(nsAString& aShortcut) {
#if defined(XP_WIN)
Expand Down Expand Up @@ -1996,7 +2002,7 @@ static void ReflectSkeletonUIPrefToRegistry(const char* aPref, void* aData) {
bool shouldBeEnabled =
Preferences::GetBool(kPrefPreXulSkeletonUI, false) &&
Preferences::GetBool(kPrefBrowserStartupBlankWindow, false) &&
Preferences::GetBool(kPrefDrawTabsInTitlebar, false);
LookAndFeel::DrawInTitlebar();
if (shouldBeEnabled && Preferences::HasUserValue(kPrefThemeId)) {
nsCString themeId;
Preferences::GetCString(kPrefThemeId, themeId);
Expand Down Expand Up @@ -2025,8 +2031,9 @@ static void SetupSkeletonUIPrefs() {
Preferences::RegisterCallback(&ReflectSkeletonUIPrefToRegistry,
kPrefBrowserStartupBlankWindow);
Preferences::RegisterCallback(&ReflectSkeletonUIPrefToRegistry, kPrefThemeId);
Preferences::RegisterCallback(&ReflectSkeletonUIPrefToRegistry,
kPrefDrawTabsInTitlebar);
Preferences::RegisterCallback(
&ReflectSkeletonUIPrefToRegistry,
nsDependentCString(StaticPrefs::GetPrefName_browser_tabs_drawInTitlebar()));
}

# if defined(MOZ_LAUNCHER_PROCESS)
Expand Down
11 changes: 5 additions & 6 deletions widget/LookAndFeel.h
Original file line number Diff line number Diff line change
Expand Up @@ -246,12 +246,6 @@ class LookAndFeel {
*/
GTKCSDAvailable,

/*
* A boolean value indicating whether GTK+ system titlebar should be
* disabled by default.
*/
GTKCSDHideTitlebarByDefault,

/*
* A boolean value indicating whether client-side decorations should
* contain a minimize button.
Expand Down Expand Up @@ -509,6 +503,11 @@ class LookAndFeel {
*/
static bool GetEchoPassword();

/**
* Whether we should be drawing in the titlebar by default.
*/
static bool DrawInTitlebar();

/**
* The millisecond to mask password value.
* This value is only valid when GetEchoPassword() returns true.
Expand Down
Loading

0 comments on commit f317384

Please sign in to comment.