Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added customize title bar options #212

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Added customize title bar options #212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d116fa8
Added customize title bar options
UtiluMark Aug 31, 2016
62bc0d3
Short way to get app.update.channel + beta version
UtiluMark Sep 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 58 additions & 9 deletions extension/chrome/content/nightly.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,36 @@ variables: {
return this._appInfo;
},

get titleversion() { return this.tabtitle + ' - ' + this.vendor + ' ' + this.name + ' ' + this.versionchannel + ' (' + this.appbuildid + ')'; },
get defaulttitle() { return nightlyApp.defaultTitle; },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to review if you wouldn't have moved around existing lines. Another commit would have been good for that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also moving those lines would have made sense if those would be in alphabetical order now. But that is still not the case. Mind doing that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Title bar Variables are grouped per kind now.
I don't think an alphabetical order would provide a clear overview of the different variables with (partly) similar values (and if you would sort alphabetically, which row would you sort alphabetically? Variable? Description? Value?).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Common identifiers could be used first eg. titleDefault, titleTab. But whatever we use then other related variables like for tabs in this case are no longer grouped together. So whatever you do it will not be perfect.

get tabtitle() { return nightlyApp.tabTitle; },
get appid() this.appInfo.ID,
brandname: null,
get vendor() {
// Fix for vendor not being set in Mozilla Thunderbird
return this.appInfo.name == "Thunderbird" && this.appInfo.vendor == "" ? "Mozilla" : this.appInfo.vendor;
},
get name() this.appInfo.name,
get version() this.appInfo.version,
get appbuildid() this.appInfo.appBuildID,
get platformbuildid() this.appInfo.platformBuildID,
get versionpretty() { return nightly.makeVersionPretty(this.version); },
get displayversion() {
var ver = this.version;
try {
// The import is only required in Mozilla Thunderbird
Components.utils.import("resource://gre/modules/AppConstants.jsm");
if (AppConstants.MOZ_APP_VERSION_DISPLAY) { ver = AppConstants.MOZ_APP_VERSION_DISPLAY; }
}
catch (e) {}
return ver;
},
get channel() { return nightly.updateChannel(); },
get channelpretty() { return nightly.updateChannelPretty(); },
get betaversion() { return nightly.betaVersion(this.version, this.displayversion); },
get versionchannel() { return nightly.versionAndChannel(this.version, this.displayversion); },
get platformversion() this.appInfo.platformVersion,
get geckobuildid() this.appInfo.platformBuildID,
get geckoversion() this.appInfo.platformVersion,
get platformbuildid() this.appInfo.platformBuildID,
get appbuildid() this.appInfo.appBuildID,
get changeset() { return nightly.getChangeset(); },
brandname: null,
get useragent() navigator.userAgent,
get locale() {
var registry = Components.classes["@mozilla.org/chrome/chrome-registry;1"]
Expand All @@ -38,11 +54,9 @@ variables: {
get os() this.appInfo.OS,
get processor() this.appInfo.XPCOMABI.split("-")[0],
get compiler() this.appInfo.XPCOMABI.split(/-(.*)$/)[1],
get defaulttitle() { return nightlyApp.defaultTitle; },
get tabscount() { return nightlyApp.tabsCount; },
get tabtitle() { return nightlyApp.tabTitle; },
profile: null,
toolkit: "cairo",
profile: null,
get tabscount() { return nightlyApp.tabsCount; },
flags: ""
},

Expand Down Expand Up @@ -259,6 +273,41 @@ parseHTML: function(url, callback) {
frame.contentDocument.location.href = url;
},

makeVersionPretty: function(ver) {
ver = ver.replace('0a2','0.0.0');
ver = ver.replace('0a1','0.0.0');
while (ver.match(new RegExp('\\.','g')).length < 3) { ver += '.0'; }
return ver;
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the benefit of that? It would not allow to see what version is actually in use. I don't think this is useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function provides an additional variable, in which all version numbers have the same format (x.x.x.x). So is doesn't change version 2.0.0.20, but for example version 48.0 becomes 48.0.0.0 (so it's easier to see you're not running for example 48.0.1) and version 48.0.2 becomes 48.0.2.0, etc.
a2 means Aurora and a1 means Nighty, so the Channel variable already provides this in a more clear way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't see a benefit here. We should keep the version numbers as used by Firefox and should not modify them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just an optional additional variable with the version number in a fixed format, next to the variable with the unmodified (non-fixed) format as used by Firefox (etc.), for those who prefer to display the version number in a fixed format.

Or do you want me to remove this additional variable from this PR?

updateChannel: function() {
return Services.prefs.getCharPref("app.update.channel");
},
updateChannelPretty: function() {
var channel = nightly.updateChannel();
if (channel == 'release') { channel = 'Release' }
else if (channel == 'esr') { channel = 'ESR' }
else if (channel == 'beta') { channel = 'beta' }
else if (channel == 'aurora') { channel = 'Aurora' }
else if (channel == 'nightly') { channel = 'Nighty' }
else if (channel == 'default') { channel = 'Default' }
else channel = '';
return channel;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar question as above. Is that method useful?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function provides an additional variable with a more readable channel name, with or in uppercase where needed, to show in the title bar. I prefer this above the all lowercase channel name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What makes it better readable? We could implement a general "make first letter uppercase" checkbox instead for all variable types later. But I don't see that it is necessary for all of our users.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A general "make first letter uppercase" would result in for example Esr instead of ESR, also it would make unwanted changes to for example the locale by changing en-US to En-US, so I don't think that is ideal.

},
betaVersion: function(ver, displayversion) {
var beta = displayversion.replace(ver + 'b','');
if (beta == displayversion) { beta = ''; };
return beta;
},
versionAndChannel: function(ver, displayversion) {
var channel = nightly.updateChannelPretty();
var beta = nightly.betaVersion(ver, displayversion);
if (channel == 'Release' || channel == 'Default') { channel = ''; }
if (channel == 'beta' && beta != '') { channel += ' ' + beta; }
ver = nightly.makeVersionPretty(ver);
if (channel != '') { ver += ' ' + channel; }
return ver;
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar as above. I'm not such a fun of all this combined strings. It will be better to allow the user to configure it in any order he/she wants.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function provides an additional variable, which displays the channel only when it's special (ESR, beta, Aurora, Nighty), but otherwise (Release, Default) not. In that case this variable only displays the version number.
This is impossible to do with any existing variable.

Also this function displays the beta revision (for example "beta 9") only in case of the beta channel.
This is also impossible to do with the separate beta variable, since that variable might be empty "" (if not in the beta channel), and doesn't include a space when filled "9".
"${Channel} ${BetaRevision}" would also display "beta 9" when in the beta channel, but it displays for example "aurora" (note the unwanted space at the end, caused by ${BetaRevision} being empty) in any other channel.

So I think this additional variable is useful, without combining too many strings.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that we break down variables to every little fraction. We should have application name, version, channel which should cover this all like:

Firefox 51.0a1 (nightly)
Firefox 49.0b3 (beta)
Firefox 45.4.0 (esr)

Lets not overcomplicate everthing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's ${Name} ${DisplayVersion} (${Channel})


pastebinAboutSupport: function() {
nightly.parseHTML("about:support", function(doc) {
var contents = doc.getElementById("contents");
Expand Down
31 changes: 19 additions & 12 deletions extension/chrome/content/titlebar/customize.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,26 +83,33 @@ init: function(aEvent)

paneTitle.bundle=document.getElementById("variablesBundle");

paneTitle.addVariable("AppBuildID");
paneTitle.addVariable("TitleVersion");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This completely messed-up the alphabetical order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order of the variables is per kind now, to make it easier to pick the preferred values between the various similar values to display in the title bar.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as said above. I would prefer to discuss this separately in a different issue if that is necessary to do for the ui. Also adding new variables and changing so many existing ones in a single commit makes it very hard to review. In the future you should try to splitup individual changes across different commits.

paneTitle.addVariable("DefaultTitle");
paneTitle.addVariable("TabTitle");
paneTitle.addVariable("AppID");
paneTitle.addVariable("BrandName");
paneTitle.addVariable("Vendor");
paneTitle.addVariable("Name");
paneTitle.addVariable("Version");
paneTitle.addVariable("VersionPretty");
paneTitle.addVariable("DisplayVersion");
paneTitle.addVariable("Channel");
paneTitle.addVariable("ChannelPretty");
paneTitle.addVariable("BetaVersion");
paneTitle.addVariable("VersionChannel");
paneTitle.addVariable("PlatformVersion");
paneTitle.addVariable("PlatformBuildID");
paneTitle.addVariable("AppBuildID");
paneTitle.addVariable("Changeset");
paneTitle.addVariable("Compiler");
paneTitle.addVariable("DefaultTitle");
paneTitle.addVariable("GeckoVersion");
paneTitle.addVariable("UserAgent");
paneTitle.addVariable("Locale");
paneTitle.addVariable("Name");
paneTitle.addVariable("OS");
paneTitle.addVariable("PlatformBuildID");
paneTitle.addVariable("PlatformVersion");
paneTitle.addVariable("Processor");
paneTitle.addVariable("Compiler");
paneTitle.addVariable("Toolkit");
paneTitle.addVariable("Profile");
paneTitle.addVariable("TabsCount");
paneTitle.addVariable("TabTitle");
paneTitle.addVariable("Toolkit");
paneTitle.addVariable("UserAgent");
paneTitle.addVariable("Vendor");
paneTitle.addVariable("Version");

paneTitle.setupTree();
},

Expand Down
23 changes: 14 additions & 9 deletions extension/chrome/locale/en-US/variables.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,29 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
#
variable.TitleVersion.description=Application Title and Version
variable.DefaultTitle.description=Default Application Title
variable.TabTitle.description=Current Tab's Title
variable.AppID.description=Application Identifier
variable.BrandName.description=Application Brand Name
variable.Vendor.description=Application Vendor
variable.Name.description=Application Name
variable.Version.description=Application Version
variable.VersionPretty.description=Application Version Pretty
variable.DisplayVersion.description=Application Display Version
variable.Channel.description=Application Update Channel
variable.ChannelPretty.description=Application Update Channel Pretty
variable.BetaVersion.description=Application Beta Version
variable.VersionChannel.description=Application Version and Channel
variable.PlatformVersion.description=XUL Platform Version
variable.PlatformBuildID.description=XUL Platform Build Identifier
variable.AppBuildID.description=Application Build Identifier
variable.Changeset.description=Built from that changeset
variable.PlatformBuildID.description=XUL Platform Build Identifier
variable.PlatformVersion.description=XUL Platform Version
variable.GeckoBuildID.description=Gecko Build Identifier
variable.GeckoVersion.description=Gecko Version
variable.BrandName.description=Application Brand Name
variable.UserAgent.description=User Agent String
variable.Locale.description=Current Locale
variable.OS.description=Compilation OS
variable.Processor.description=Compilation Processor
variable.Compiler.description=Compiler
variable.DefaultTitle.description=Default Application Title
variable.TabTitle.description=Current Tab's Title
variable.Profile.description=Current Profile
variable.Toolkit.description=Graphics Toolkit
variable.TabsCount.description=Count of visible tabs
variable.Profile.description=Current Profile
variable.TabsCount.description=Number of visible tabs
4 changes: 2 additions & 2 deletions extension/chrome/skin/titlebar/titlebar.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

#variableTree {
height: 17em;
height: 23em;
}

#NightlyTesterOptions {
width: 42.75em;
width: 56em;
}