369030 - mainWindow title has no info about view

Status: VERIFIED FIXED

(Calendar :: General, enhancement)

Description

[Markus Adrario [:Taraman]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1;en-US;rv:1.9a2pre) Gecko/20070108 Calendar/0.4a1

In Firefox, the Window Title holds the title of the currntly viewed page + " - Mozilla Firefox"
In Thunderbird, it's the name of the mail-folder + " - Thunderbird"
So I thought it would be a good thing to have something similar in Sunbird and Lightning.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Markus Adrario [:Taraman]]

Attachment: adds current view Title to the mainWindow

With this patch the Actual view title is set to be the window title and the short brand name is appended.

e.g. if you view February 2007, the Sunbird window title changes to "February 2007 - Calendar"
It also works for lightning, where " - Thunderbird" is appended as you expect it in the Mail Application.

Comment 2

[Markus Adrario [:Taraman]]

Attachment: Screenshot of the new look

Comment 3

[Joey Minta]

Comment on attachment 253697 [details] [diff] [review]
adds current view Title to the mainWindow

Again, please ask for review so that we can get these sweet patches into the tree! :-)

drive-by comments:
1.) Fix indenting.
2.) Wrap lines to be < 80 chars.

Comment 4

[Markus Adrario [:Taraman]]

Attachment: Adds view-Info to mainWindow-Title

fixed indentation and line-length...

Comment 5

[Matthew (lilmatt) Willis]

Comment on attachment 253759 [details] [diff] [review]
Adds view-Info to mainWindow-Title

Removing review request until this makes it through ui-r

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 253759 [details] [diff] [review]
Adds view-Info to mainWindow-Title

ui-review=mvl

Comment 7

[Matthew (lilmatt) Willis]

Comment on attachment 253759 [details] [diff] [review]
Adds view-Info to mainWindow-Title

>+		     var sbs = Components.classes["@mozilla.org/intl/stringbundle;1"]
>+				 .getService(Components.interfaces.nsIStringBundleService);
Align the .getService dot with the C.c dot please.

>+                    var props = sbs.createBundle("chrome://branding/locale/brand.properties");
Since this is the brand stringbundle, I would suggest renaming the variable to something else.  We often use props for the calendar.properties stringbundle elsewhere in the code, and this might be confusing.  I suggest "brand". 

>+		     document.title = document.getAnonymousElementByAttribute(this, "anonid", 
>+				 "title-0").label+" - "+props.GetStringFromName("brandShortName");
This needs to be split up to be more readable.  

My suggestion is to assign the doc.getAnonElement call to its own variable, and use that in the assignment to doc.title.  Also, please put spaces around the + when concatenating strings.

These are all style nita.  r1=lilmatt with those fixed.

Please post another version for whomever does r2.

Comment 8

[Markus Adrario [:Taraman]]

Attachment: changes from r1

I changed the things lilmatt commented on.
Thx for the good hints to a mozilla programming newbie.

Comment 9

[Joey Minta]

Comment on attachment 254351 [details] [diff] [review]
changes from r1

There are some tabs in here that need to be converted to spaces (No tabs allowed in js, xul, or xml files).

+		     var viewTitle = document.getAnonymousElementByAttribute(this, "anonid", "title-0")
You should have a semicolon at the end of this line.  But really, wouldn't it be easier to just get aNameArray[2], rather than reading off .label from the element?  That would avoid the need for this whole line.

r2=jminta with the tabs fixed, and either switching to aNameArray[2], or someone explaining to me why we can't.  Thanks for the patch!

Comment 10

[Markus Adrario [:Taraman]]

Attachment: changes requested by Joey Minta

Thanks for the hint Joey!
Why make it easy when you can take the difficult way.
aNameArray[2] works fine, I just didn't see it.

fixed that, which obsoletes the missing ; and fixed the tabs.

Comment 11

[Joey Minta]

Great work.  Patch checked in.
  calendar/base/content/calendar-decorated-base.xml: 1.13 -> 1.14

Comment 12

[Omar Bajraszewski]

In month view: I can see i.e. "February 2007". In day view I can see i.e. "Saturday". But what's the date? IMO it should look like "Saturday, 17 Feb" or something like it

Comment 13

[Damian Szczepanik]

verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20070216 Calendar/0.4a1

Comment 14

[Damian Szczepanik]

Litmus testcase 3009 created