Add new variable for visible tabs #173

KOLANICH · 2014-09-29T19:36:43Z

Now with useless resource string (why not to remove descriptions, tag names are already self-describing) and variants for SeaMonkey and Thunderbird.

This fixes issue #171.

whimboo · 2014-09-29T20:44:59Z

extension/chrome/content/browser.js

+get tabsCount() {
+  var tabbrowser = document.getElementById("content");
+  return tabbrowser.visibleTabs.length;
+},


Just a little update to do here... would you mind putting this getter before tabsTitle in all 3 files? That way we have it alphabetically sorted.

whimboo · 2014-09-29T20:46:57Z

Those changes look great! Thanks for fixing this @KOLANICH! @xabolcs or @tonymec, would one of you both mind testing it for each of those products?

xabolcs · 2014-09-29T22:55:02Z

The change works with

Mozilla/5.0 (X11; Linux x86_64; rv:35.0) Gecko/20100101 Thunderbird/35.0a1 ID:20140922030203 CSet: 9717f3be5a20

But the strings changed and other locales than en-US miss the new description.

xabolcs · 2014-09-29T22:58:42Z

extension/chrome/locale/en-US/variables.properties

@@ -22,3 +22,4 @@ 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


@KOLANICH please add this line to all variables.properties files in the repository (extension/chrome/locale/*/variables.properties). It's OK to add as unchanged i.e. untranslated strings.

The guys at BabelZilla will do the rest! :)

whimboo · 2014-09-30T07:01:54Z

Good catch. Thanks @xabolcs! That's indeed something I missed. If all is fine and if it works in SeaMonkey too, feel free to merge the PR.

tonymec · 2014-10-01T11:06:54Z

I haven't yet tested this new NTT version, but does it support all the variables supported by the development version of the add-on, and in particular ${PlatformChangeset}, which, on Thunderbird and SeaMonkey but not on Firefox, differs from ${Changeset}? (see https://wiki.mozilla.org/QA/Automation/Projects/Addons/NightlyTesterTools#Variables ): on Firefox they both refer to the mozilla-{central|aurora|beta|release} changeset but on Thunderbird and SeaMonkey ${Changeset} is the comm-{central|aurora|beta|release} changeset.

If it doesn't, I'm not going to install it into my usual profile, where I already have the Tab Counter extension to display my current number of tabs on a toolbar.

Also, I'd like not to be the only one who is conscious of the existence of the above-mentioned mozillawiki page and keeps it up-to-date.

xabolcs · 2014-10-01T11:33:07Z

@tonymec, this pull request adds a new variable, ${TabsCount}, and doesn't support those variables you mentioned, so it's not recommended to install it into your usual profile.

xabolcs · 2014-10-01T11:38:16Z

extension/chrome/locale/zh-CN/variables.properties

@@ -22,3 +22,4 @@ variable.DefaultTitle.description=默认应用程序标题
 variable.TabTitle.description=当前标签标题
 variable.Profile.description=当前配置文件
 variable.Toolkit.description=图形工具包
+variable.TabsCount.description=Count of tabs


Thanks!
Please use the same string (i.e. Count of visible tabs) as in en-US locale.
Simply just append the en-US line (variable.TabsCount.description=Count of visible tabs) to the other variables.properties files.

One more commit please for sv-SE: extension/chrome/locale/sv-SE/variables.properties!

tonymec · 2014-10-01T12:00:17Z

@xabolcs : IMHO when adding variables we shouldn't have to choose between one and the other. Couldn't this enhancement have been based on the set of variables already supported by the development version at AMO? Is it already too late to make it so? I'm already using both ${Changeset} and ${PlatformChangeset} in my customized clipboard-or-textbox ID string. With MR-Tech Toolkit (when it worked on SeaMonkey) I used to have the number of tabs in the title. Now it's one or the other, but not both, and since there exists a Tab Counter extension, to me having the ability to autofill both changesets is more important.

xabolcs · 2014-10-01T12:42:20Z

@tonymec, this PR should be based on master. But I could merge this into your favorite all-unmerged-pulls development branch later today. :)

xabolcs · 2014-10-01T12:50:51Z

@KOLANICH thanks for the update!

@whimboo, PR is complete now, could you provide a commit string, please?
My proposed one is: Add tabs counter to titlebar customization (#152).

I'll test this updated pull request later today with Firefox, Thunderbird and Seamonkey.
If it's all OK I would land it.

tonymec · 2014-10-01T13:02:33Z

@xabolcs : ah, good. I just noticed mozilla/master cannot be merged into tonymec/master, so I'm gonna remove my forked repo if I find how. Someday I may recreate it then from mozilla.

whimboo · 2014-10-01T13:13:35Z

Regarding the wiki page I think we should get all this content transferred over to this repositories wiki. Then both code and documentation are side-by-side.

@xabolcs the commit message looks fine to me. If all works fine to you, we can get this landed.

xabolcs · 2014-10-01T18:28:21Z

extension/chrome/content/suite.js

@@ -27,6 +27,9 @@ get defaultTitle() {
  }
 },

+get tabsCount() {
+  return gBrowser.visibleTabs.length;


This throws because the lack of tab groups.

Timestamp: 10/01/2014 08:19:44 PM Error: TypeError: gBrowser.visibleTabs is undefined Source File: chrome://nightly/content/suite.js Line: 31

gBrowser.tabs.length would do the trick, but...

Very strange. According to the docs, gBrowser is tabbrowser element, and it must have visibleTabs property.

Has SeaMonkey a different implementation? It might be...

It really doesn't have. Is SM mozilla project? It seems it is third-party project...

Its a product driven by the community. Similar to Thunderbird now.

The tabbrowser implementations of SeaMonkey and Firefox are different. So different, in fact, that there is no hope of ever getting the Tab Mix Plus extension to work for SeaMonkey.

SeaMonkey, Firefox and Thunderbird are partly based on the same sources (for Gecko, Toolkit, NSS, NSPR etc.) but each of them has its own UI on top of those backends, and there are many differences between these three frontends; sometimes subtle, elusive ones; other times (as with Firefox's and SeaMonkey's tabbrowser implementations) the frontends are very different.

See among others https://developer.mozilla.org/en-US/Add-ons/SeaMonkey_2 about some differences between Firefox and SeaMonkey.

xabolcs · 2014-10-01T18:37:07Z

@KOLANICH, what about to rework this PR to support "(all) tabs count" (gBrowser.tabs) and "tabs count in current group" (gBrowser.visibleTabs)?

KOLANICH · 2014-10-01T19:45:05Z

what about to rework this PR to support "(all) tabs count" (gBrowser.tabs) and "tabs count in current group" (gBrowser.visibleTabs)?

Not yet. At first we should finish this issue: add displaying of count of visible tabs to the addon.

xabolcs · 2014-10-02T08:23:53Z

Not yet. At first we should finish this issue: add displaying of count of visible tabs to the addon.

OK.

xabolcs · 2014-10-02T08:27:19Z

extension/chrome/content/browser.js

@@ -13,6 +13,10 @@ get defaultTitle() {
  return tabbrowser.getWindowTitleForBrowser(tabbrowser.mCurrentBrowser);
 },

+get tabsCount() {
+  var tabbrowser = document.getElementById("content");
+  return tabbrowser.visibleTabs.length;


visibleTabs only available for Gecko 2 and up, so please use mTabs (this doesn't exists anymore in recent Firefoxes) or browsers (this is still available) here for Fx 3.6.
For example:

return tabbrowser.visibleTabs ? tabbrowser.visibleTabs.length : tabbrowser.browsers.length;

I would really purpose that we finally get rid of this old cruft and kill support for Firefox <4.0. What value does it bring to us to keep maintaining a version really no-one is using or better should use anymore? It only makes our code way too complex for no value. IMO it's wasted time. I'm happy to file a new issue for this for discussion.

See #161.

It only makes our code way too complex for no value.

It's already too complex, without these old cruft.
So let refactor the code with the same compatibility/functionality first. But this is off-topic here.

xabolcs · 2014-10-02T23:59:20Z

Thanks for the update @KOLANICH!

I see you always rewrite your commits. May I ask you to amend the commit message to Add tabs counter to titlebar customization (#152)?

If this and the

return tabbrowser.visibleTabs ? tabbrowser.visibleTabs.length : tabbrowser.browsers.length;

is done for browser.js then I could merge this PR with fast-forward merge.
Thanks!

KOLANICH · 2014-10-03T16:39:06Z

return tabbrowser.visibleTabs ? tabbrowser.visibleTabs.length : tabbrowser.browsers.length;

Why should we support old versions? Especially SUCH old versions? If a person uses such an old version of a browser, this means that he doesn't care about it, because he either doesn't care about security or doesn't know about the danger. This means that he is ordinary user, not IT geek. IT geeks (target group of this addon) understand that using old versions with tons of vulnerabilites is ultimately insecure.

Also one more question. Why do we use History.md ? Is git log insufficient to generate changelog automatically?

tonymec · 2014-10-03T22:58:33Z

There are in NTT a lot of features, many of which are useful for any user, not only IT geeks. This said, the older a version, the less we need to support it; but NTT always did, and IMHO should continue to, support all three of Firefox, Thunderbird and SeaMonkey. The tabbrowsers of SeaMonkey and Firefox have drifted apart since the day Firefox was forked from the Suite: does tabbrowser.visibleTabs exist in SeaMonkey (which has no notion of "tab groups")? If it doesn't, we ought to get the number of tabs some other way. For instance by counting elements of tabbrowser.browsers

DOM Inspector tells me that the elements of interest to us are structured as follows in SeaMonkey 2.32a1 (built on Gecko 35.0a1) (following the chain from parent to child, and omitting siblings): &35;document > window#main-window > hbox > vbox#appcontent > hbox#browser > tabbrowser#content > tabbox > tabpanels.plain > notificationbox.browser-notificationbox > browser > &35;document > HTML > etc. There is one XUL <tabpanels class="plain"> element containing as many XUL <notificationbox> elements as there are tabs. AFAICT, the two <#document> elements are the chrome and the content.

whimboo · 2014-10-06T11:05:19Z

Also one more question. Why do we use History.md ? Is git log insufficient to generate changelog automatically?

It has simply been proven that this file is kinda helpful when users want to have a look what has been changed. There is no need for them to dig through the code, find appropriate tags, and create a local diff. The update of this file is kinda easy via git-extras.

whimboo · 2014-10-06T11:06:33Z

Oh and looks like there is one missing piece still to be done. I will leave it up to @xabolcs to finally review and land it. Thanks!

xabolcs · 2014-10-06T11:10:46Z

@whimboo, yes one nit is missing, but could be done at landing time.

xabolcs · 2014-10-06T22:20:39Z

This PR was landed as 0553dec.

xabolcs · 2014-10-07T08:46:28Z

@whimboo removed this from the 3.8 milestone 10 hours ago

Hmm. I think I added the bad reference to commit message.
As I wrote earlier, this PR could be part of the series of fixing issue #152.
What do you think, @whimboo?

whimboo · 2014-10-07T08:50:29Z

I wouldn't worry about that detail. We can keep the reference to #171, which is the issue this PR fixed. So #152 is more like a meta bug.

xabolcs · 2014-10-07T08:52:14Z

OK, I will leave a comment in #152, that the tabs count part was implemented here.
Thanks!

KOLANICH mentioned this pull request Sep 29, 2014

Add tabs count as template variable for titlebar customization #171

Closed

whimboo reviewed Sep 29, 2014
View reviewed changes

xabolcs reviewed Sep 29, 2014
View reviewed changes

xabolcs reviewed Oct 1, 2014
View reviewed changes

xabolcs reviewed Oct 2, 2014
View reviewed changes

Added tabs counter

8c3c984

whimboo changed the title ~~Added tabs counter~~ Add new variable for visible tabs Oct 6, 2014

xabolcs pushed a commit to xabolcs/nightlytt that referenced this pull request Oct 6, 2014

Add new variable for visible tabs (mozilla#173)

ea02db1

xabolcs added this to the 3.8 milestone Oct 6, 2014

xabolcs pushed a commit to xabolcs/nightlytt that referenced this pull request Oct 6, 2014

Add new variable for visible tabs (mozilla#173)

0553dec

xabolcs closed this Oct 6, 2014

whimboo removed this from the 3.8 milestone Oct 6, 2014

KOLANICH deleted the feature-TabsCount branch October 7, 2014 06:21

xabolcs assigned KOLANICH Oct 7, 2014

xabolcs modified the milestone: 3.8 Oct 7, 2014

xabolcs mentioned this pull request Oct 7, 2014

[customize-titlebar] Tabs, groups and windows count #152

Open

Add new variable for visible tabs #173

Add new variable for visible tabs #173

Conversation

KOLANICH commented Sep 29, 2014

Choose a reason for hiding this comment

whimboo commented Sep 29, 2014

xabolcs commented Sep 29, 2014

Choose a reason for hiding this comment

whimboo commented Sep 30, 2014

tonymec commented Oct 1, 2014

xabolcs commented Oct 1, 2014

Choose a reason for hiding this comment

tonymec commented Oct 1, 2014

xabolcs commented Oct 1, 2014

xabolcs commented Oct 1, 2014

tonymec commented Oct 1, 2014

whimboo commented Oct 1, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xabolcs commented Oct 1, 2014

KOLANICH commented Oct 1, 2014

xabolcs commented Oct 2, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xabolcs commented Oct 2, 2014

KOLANICH commented Oct 3, 2014

tonymec commented Oct 3, 2014

whimboo commented Oct 6, 2014

whimboo commented Oct 6, 2014

xabolcs commented Oct 6, 2014

xabolcs commented Oct 6, 2014

xabolcs commented Oct 7, 2014

whimboo commented Oct 7, 2014

xabolcs commented Oct 7, 2014