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.

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

Bug 726560 - Add support of Panorama group name (FF10+) for titlebar customization #19

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
60 changes: 60 additions & 0 deletions extension/chrome/content/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ repository: ['mozilla-central','mozilla-aurora'],

storedTitle: document.documentElement.getAttribute("titlemodifier"),

LAST_SESSION_GROUP_NAME_IDENTIFIER: "nightlytt-last-session-group-name",
_lastSessionGroupName: "",

get defaultTitle() {
var tabbrowser = document.getElementById("content");
return tabbrowser.getWindowTitleForBrowser(tabbrowser.mCurrentBrowser);
Expand All @@ -51,6 +54,32 @@ get tabTitle() {
return tabbrowser.mCurrentBrowser.contentTitle;
},

get activeTabGroupName() {
Copy link
Contributor

Choose a reason for hiding this comment

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

@ttaubert - would you mind checking the code if it's getting the group title the correct way?

// TabView isn't implemented or initialized
if (!TabView || !TabView._window)
return nightlyApp._lastSessionGroupName;
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if no tab group exists anymore? Would we carry around the last name forever? Or is there at least one group in any case?



// We get the active group this way, instead of querying
// GroupItems.getActiveGroupItem() because the tabSelect event
// will not have happened by the time the browser tries to
// update the title.
let groupItem = null;
let activeTab = window.gBrowser.selectedTab;
let activeTabItem = activeTab._tabViewTabItem;

if (activeTab.pinned) {
// It's an app tab, so it won't have a .tabItem. However, its .parent
// will already be set as the active group.
groupItem = TabView._window.GroupItems.getActiveGroupItem();
} else if (activeTabItem) {
groupItem = activeTabItem.parent;
}

// groupItem may still be null, if the active tab is an orphan.
return groupItem ? groupItem.getTitle() : "";
},

init: function()
{
var brandbundle = document.getElementById("bundle_brand");
Expand All @@ -66,6 +95,24 @@ init: function()

tabbrowser.updateTitlebar = nightly.updateTitlebar;
tabbrowser.addEventListener("DOMTitleChanged", nightly.updateTitlebar, false);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • Re: sessionstore. It should already have the name of the group included per default. Can't we simply retrieve it?

If I move these lines some line below, into the if ... else then getting and setting of tabGroupTitle would be in sync.
After that I wouldn't understand Your question.
Would You mind to write some details about Your question? Because I don't really know why are You asking that.

Copy link
Contributor

Choose a reason for hiding this comment

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

The question was about our own sessionstore entry for NTT. Per default the sessionstore.js file will contain the list of all the windows, tabs, and groups. So if we would query the session store service we probably could retrieve the last active group on startup, whereby we wouldn't have to store our own property.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Still not clear. :)

Are You talking about L#44, LAST_SESSION_GROUP_NAME_IDENTIFIER: "nightlytt-last-session-group-name", ?
I wouldn't like to collide TabView's property if we are before Bug 682996. In that case I plan simply use TabView's stuff, and do not save / restore ours.

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 think that we should care about code before bug 682996 has been fixed. It was fixed for Firefox 10 which we still support on the ESR branch. Releases before we do not support anymore. 3.6.x is an exception but will also be dropped soon.

// Listening to Bug 659591 (landed in FF7) - instead "domwindowclosed" (see Bug 655269),
// to store active group's name for showing at next startup
window.addEventListener("SSWindowClosing", function NightlyTT_onWindowClosing() {
window.removeEventListener("SSWindowClosing", NightlyTT_onWindowClosing, false);
nightlyApp.saveActiveGroupName(window);
}, false);

// grab the last used group title
// use TabView's property if we are before Bug 682996 (landed in FF10)
nightlyApp._lastSessionGroupName = (TabView && TabView._lastSessionGroupName)
? TabView._lastSessionGroupName
: Cc["@mozilla.org/browser/sessionstore;1"]
.getService(Ci.nsISessionStore)
.getWindowValue(
window,
nightlyApp.LAST_SESSION_GROUP_NAME_IDENTIFIER
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the ternary operator here. It makes it kinda hard to read. Also please better align those rows.

},

openURL: function(url)
Expand All @@ -87,6 +134,19 @@ openNotification: function(id, message, label, accessKey, callback) {
message, "urlbar", action, null, options);
},

// Function: saveActiveGroupName
// Saves the active group's name for the given window.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind starting to use JSdoc comments? http://code.google.com/p/jsdoc-toolkit/w/list

saveActiveGroupName: function NightlyTT_saveActiveGroupName(win) {
let groupName = nightlyApp.activeTabGroupName;
Cc["@mozilla.org/browser/sessionstore;1"]
.getService(Ci.nsISessionStore)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please get a reference to this service once and store it globally. When will those values be deleted from the session store file?

.setWindowValue(
win,
nightlyApp.LAST_SESSION_GROUP_NAME_IDENTIFIER,
groupName
);
},

setCustomTitle: function(title)
{
document.getElementById("content").ownerDocument.title = title;
Expand Down
5 changes: 5 additions & 0 deletions extension/chrome/content/nightly.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ variables: {
get compiler() this.appInfo.XPCOMABI.split("-")[1],
get defaulttitle() { return nightlyApp.defaultTitle; },
get tabtitle() { return nightlyApp.tabTitle; },
get activetabgroupname() { return nightlyApp.activeTabGroupName || null; },
Copy link
Contributor

Choose a reason for hiding this comment

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

tabGroupTitle?

profile: null,
toolkit: "cairo",
flags: ""
Expand Down Expand Up @@ -107,6 +108,10 @@ showAlert: function(id, args) {

init: function() {
window.removeEventListener("load", nightly.init, false);
setTimeout(nightly.initLazy,800);
},

initLazy: function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind moving this out to another pull request? I'm not yet, if this will cause side-effects by lazily loading / executing some of the code.

var prefs = Components.classes["@mozilla.org/preferences-service;1"]
.getService(Components.interfaces.nsIPrefService);
nightly.preferences = prefs.getBranch("nightly.")
Expand Down
1 change: 1 addition & 0 deletions extension/chrome/content/titlebar/customize.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ init: function()

paneTitle.addVariable("DefaultTitle");
paneTitle.addVariable("TabTitle");
paneTitle.addVariable("ActiveTabGroupName");
paneTitle.addVariable("AppID");
paneTitle.addVariable("Vendor");
paneTitle.addVariable("Name");
Expand Down
1 change: 1 addition & 0 deletions extension/chrome/locale/en-US/variables.properties
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,6 @@ variable.Processor.description=Compilation Processor
variable.Compiler.description=Compiler
variable.DefaultTitle.description=Default Application Title
variable.TabTitle.description=Current Tab's Title
variable.ActiveTabGroupName.description=Active TabView group name - may be empty in rare cases
Copy link
Contributor

Choose a reason for hiding this comment

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

Please reduce the variable to 'TabGroup' so we stay in sync with 'TabTitle' and others. Also please kill the last part of the description content and use 'Tab Group' instead of 'TabView', e.g. 'Current Tab Group'.

variable.Profile.description=Current Profile
variable.Toolkit.description=Graphics Toolkit