-
Notifications
You must be signed in to change notification settings - Fork 37
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
Changes from 13 commits
a14ef5f
bcdc712
57e2da6
7dbf770
e52f841
91017e7
5f86f3d
2b7bfbf
2853a69
0b1787c
630b02e
5e5edaf
e11907e
3c7f334
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,6 +51,14 @@ get tabTitle() { | |
return tabbrowser.mCurrentBrowser.contentTitle; | ||
}, | ||
|
||
get tabGroupTitle() { | ||
try | ||
{ | ||
return nightlyApp.getTabGroupTitle(window); | ||
} | ||
catch (e) { } | ||
}, | ||
|
||
init: function() | ||
{ | ||
var brandbundle = document.getElementById("bundle_brand"); | ||
|
@@ -66,6 +74,11 @@ init: function() | |
|
||
tabbrowser.updateTitlebar = nightly.updateTitlebar; | ||
tabbrowser.addEventListener("DOMTitleChanged", nightly.updateTitlebar, false); | ||
|
||
try { // import tabGroupTitle functionality for titlebar customization | ||
Components.utils.import("resource://nightly/tabGroupTitle.jsm", nightlyApp); | ||
} | ||
catch(e) { Components.utils.reportError(e); } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure, that error report is needed. IMHO it's enough to fail silently. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we have to put this into try/catch? The file will always be available and this import should never fail. If it does it would be fatal, and the failure should really bubble up. |
||
}, | ||
|
||
openURL: function(url) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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.TabGroup.description=Current Tab Group | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As you know, we have
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please do the merge yourself and ensure that all is included. Thanks. |
||
variable.Profile.description=Current Profile | ||
variable.Toolkit.description=Graphics Toolkit |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
var EXPORTED_SYMBOLS = ["getTabGroupTitle"]; | ||
|
||
const Cc = Components.classes; | ||
const Ci = Components.interfaces; | ||
|
||
const GROUP_DATA_IDENTIFIER = "tabview-group"; | ||
|
||
let sstore = Cc["@mozilla.org/browser/sessionstore;1"].getService(Ci.nsISessionStore); | ||
|
||
|
||
/** | ||
* Simply retrieves the active tabgroup's title from SessionStore | ||
*/ | ||
function getActiveGroupName(win) { | ||
let data = "", groupTitle = ""; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: please put those on separate lines. |
||
try { | ||
data = sstore.getWindowValue(win, win.TabView.GROUPS_IDENTIFIER); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why don't you make use of ´GROUP_DATA_IDENTIFIER´ here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know! :) But it looks like this ("tabview-group") will work for Tab Groups and Simple Tab Groups too! :) |
||
if (data) { | ||
let parsedData = {}; | ||
parsedData = JSON.parse(data); | ||
let activeGroupId = parsedData.activeGroupId; | ||
data = sstore.getWindowValue(win, GROUP_DATA_IDENTIFIER); | ||
parsedData = JSON.parse(data); | ||
groupTitle = parsedData[activeGroupId].title; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you mind to add a comment what this code is doing, and why you have to parse twice? That would help a lot. Thanks. |
||
} | ||
} catch (e) { } | ||
|
||
return groupTitle; | ||
} | ||
|
||
/** | ||
* Calculates Tab Group's title for nightlyApp | ||
* @param {nsIDOMWindow} win A window which contains nightly. | ||
* | ||
* In Gecko 1.x title is set to "Undefined" as in other Apps | ||
* Before FF10 title is managed by TabView | ||
* After that we manages the title: using SessionStore and other borrowed code to generate | ||
*/ | ||
function getTabGroupTitle(win) { | ||
let nightlyApp = win.nightlyApp; | ||
let TabView = win.TabView; | ||
|
||
// TabView isn't implemented | ||
if (!("TabView" in win)) | ||
return; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
// If we are before Bug 682996, | ||
// use TabView's own implementation except it is null | ||
if (typeof(TabView.getActiveGroupName) === "function") | ||
return TabView.getActiveGroupName() || ""; | ||
|
||
// TabView isn't initialized | ||
if (!TabView._window) { | ||
return getActiveGroupName(win); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there is no legal way to modify active group and / or active group's title then caching the result of |
||
} | ||
|
||
// 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 = win.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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I would not recommend to use the hanging style. Lets move the ´else´ and ´catch´ statements into their own line. |
||
groupItem = activeTabItem.parent; | ||
} | ||
|
||
// groupItem may still be null, if the active tab is an orphan. | ||
return groupItem ? groupItem.getTitle() : ""; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see where ´getTabGroupTitle´ throws an exception. Can you please explain that? If we silently catch that, we might already want to do that in that method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is getTabGroupTitle defined in SeaMonkey? AFAIK, there are no tab groups there at all (yet). Or do we never come here in SeaMonkey (however braindead the user's actions)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you can see,
nightlyApp.getTabGroupTitle()
is only defined inbrowser.js
=== Firefox.Therefore the current implementation needs the
try ... catch
. (Of course it could be improved, to avoidtry ... catch
)BTW, only Firefox supports TabGroups (but the dev guys are going to extract it to an addon).