419816 - Task mode does not set Application title

Status: VERIFIED FIXED

(Calendar :: Lightning Only, defect)

Description

[Sven Giermann]

After switching to task mode, the application title remains as it was before.

STR:
- select "Inbox" in Mail mode
- switch to Task mode

ACTUAL RESULT:
- title remains "Inbox - Thunderbird"

EXPECTED RESULT:
- title should be "Tasks - Thunderbird" (or something else)


STR2:
- switch to Calendar mode (eg. Month view "March 2008")
- switch to Task mode

ACTUAL RESULT:
- title remains "March 2008 - Thunderbird"

EXPECTED RESULT:
- title should be "Tasks - Thunderbird" (or something else)

Comment 1

[Sven Giermann]

Attachment: change title in ltnSwitch2Task()

I chose to steal the title "Tasks" from the messageMenu, which is being set by swapPopupMenus().
This is probably not the best way, but it avoids the need for further locales...

To finally solve this there are some questions open:

- Is this the right place to set the Application title or should this go into 'mozilla/calendar/base/content/calendar-task-view.js'?

- Should we steal another locale resource or create our own 'TaskModeTitle' in 'calendar.properties'?

- Should the title also reflect the applied filters (eg. "Not Started Tasks - Thunderbird")

Also this bug should somehow depend on the desired implementation of Task mode in Sunbird (bug 405508) - setting dependency later...

Should this first workaround go into 0.8 to get a more 'complete' Task mode in this release?

Comment 2

[Simon Paquet [:sipaq]]

Comment on attachment 305986 [details] [diff] [review]
change title in ltnSwitch2Task()

> Created an attachment (id=305986) [details]
> change title in ltnSwitch2Task()
> 
> I chose to steal the title "Tasks" from the messageMenu, which is being set 
> by swapPopupMenus(). This is probably not the best way, but it avoids the 
> need for further locales...

This is not the right way to go. We have no assurance that the string value will always remain the same for the application title and the message menu.

In addition different locales may have different translations for these different items.

r- because of that.

> To finally solve this there are some questions open:
> 
> - Is this the right place to set the Application title or should this 
>   go into 'mozilla/calendar/base/content/calendar-task-view.js'?

It's probably best to send a mail to Mickey about this and ask him.

> - Should we steal another locale resource or create our own 
>   'TaskModeTitle' in 'calendar.properties'?

Don't steal locale resources if you aren't 100% sure, that what I said 
above cannot occur.

> - Should the title also reflect the applied filters (eg. "Not Started 
>   Tasks - Thunderbird")

I think that this would just add unneeded complexity.
 
> Should this first workaround go into 0.8 to get a more 'complete' Task 
> mode in this release?

We should do it right the first time, if we can afford it. This bug isn't 
serious enough to warrant a workaround.

Comment 3

[Michael Büttner (no reviews TFN)]

(In reply to comment #2)
> - Is this the right place to set the Application title or should this 
>   go into 'mozilla/calendar/base/content/calendar-task-view.js'?
This is indeed the right place to add this particular logic. calendar-task-view.js should contain the task mode itself, i.e. hosting the various controls. The switching logic should be restriced to messenger-overlay-toolbar.js.

Comment 4

[Simon Paquet [:sipaq]]

This should come for 0.9 but not for 0.8

Comment 5

[Sven Giermann]

Attachment: patch v2

This one got out of my radar...

But should definitely come for 0.9!

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 323681 [details] [diff] [review]
patch v2

>+    // change title to "Tasks"
>+    var sbs = Components.classes["@mozilla.org/intl/stringbundle;1"]
>+                        .getService(Components.interfaces.nsIStringBundleService);
>+    var brand = sbs.createBundle("chrome://branding/locale/brand.properties");
>+    document.title = calGetString("calendar", "taskModeApplicationTitle") + " - " + 
>+                     brand.GetStringFromName("brandShortName");
>+
calGetString can do more:
    document.title = calGetString("calendar", "taskModeApplicationTitle") + " - " + 
                     calGetString("brand", "brandShortName", null, "branding");




>Index: mozilla/calendar/locales/en-US/chrome/calendar/calendar.properties
...
>+taskModeApplicationTitle=Tasks
I think this string should rather land in lightning.properties or similar. While I know task mode is targeted also for sunbird, I think we can still move this string when task mode has been implemented for Sunbird.


r=philipp with comments considered.

Comment 7

[Sven Giermann]

Attachment: patch v3, Philipp's comments applied

(In reply to comment #6)
> calGetString can do more:
>     document.title = calGetString("calendar", "taskModeApplicationTitle") + " -
> " + 
>                      calGetString("brand", "brandShortName", null, "branding");

Oh, thanks.

> I think this string should rather land in lightning.properties or similar.
> While I know task mode is targeted also for sunbird, I think we can still move
> this string when task mode has been implemented for Sunbird.

I also spent some minutes thinking about the right place to add the string. I'm fine with both locations, so I moved it.

> r=philipp with comments considered.
Okay, taking r+

Comment 8

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED

Comment 9

[Andreas Treumann]

Checked in lightning build 2008060519 -> VERIFIED.