Closed Bug 419816 Opened 17 years ago Closed 16 years ago

Task mode does not set Application title

Categories

(Calendar :: Lightning Only, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: giermann, Assigned: giermann)

Details

Attachments

(1 file, 2 obsolete files)

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)
Flags: wanted-calendar0.8?
Attached patch change title in ltnSwitch2Task() (obsolete) — Splinter Review
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?
Attachment #305986 - Flags: review?
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.
Attachment #305986 - Flags: review? → review-
(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.
This should come for 0.9 but not for 0.8
Flags: wanted-calendar0.8? → wanted-calendar0.8-
Flags: wanted-calendar0.8- → wanted-calendar0.9+
Attached patch patch v2 (obsolete) — Splinter Review
This one got out of my radar...

But should definitely come for 0.9!
Attachment #305986 - Attachment is obsolete: true
Attachment #323681 - Flags: review?(philipp)
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.
Attachment #323681 - Flags: review?(philipp) → review+
(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+
Attachment #323681 - Attachment is obsolete: true
Attachment #323855 - Flags: review+
Keywords: checkin-needed
Checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 0.9
Checked in lightning build 2008060519 -> VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: