Closed Bug 954274 Opened 8 years ago Closed 8 years ago

Fix Windows Jump List code

Categories

(Instantbird :: Other, defect)

x86
Windows 7
defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clokep, Assigned: clokep)

References

Details

(Whiteboard: [0.3-blocking-final])

Attachments

(1 file, 2 obsolete files)

*** Original post on bio 841 at 2011-06-15 15:15:00 UTC ***

Various aspects of the Windows Jump List code could use a few tweaks.

1. Use "Unavailable" instead of "Away" (both the string and the status).
2. The strings are translated in [1] already, so we don't need to retranslate them in [2].
3. Use the |toFlag| function being added to [3] as part of bug 954180 (bio 746) (this will allow all statuses to be set from the command line, although we will only provide the UI for Available, Unavailable, Offline).

[1] http://lxr.instantbird.org/instantbird/source/purple/locales/en-US/status.properties
[2] http://lxr.instantbird.org/instantbird/source/instantbird/locales/en-US/chrome/instantbird/winjumplist.properties
[3] http://lxr.instantbird.org/instantbird/source/purple/purplexpcom/src/imStatusUtils.jsm
Depends on: 954054
*** Original post on bio 841 at 2011-06-15 15:17:48 UTC ***

(In reply to comment #0)
> Various aspects of the Windows Jump List code could use a few tweaks.
> 2. The strings are translated in [1] already, so we don't need to retranslate
> them in [2].
Also, we could probably drop the tooltips, they don't really seem to be used in Windows? Otherwise, we'll need a new string unavailable.tooltip.
Whiteboard: [0.3-blocking-final]
Attached patch v1.0 (obsolete) — Splinter Review
*** Original post on bio 841 as attmnt 715 at 2011-06-16 01:23:00 UTC ***

Fixing the four issues by removing the tooltips (which is the only option I see without breaking the string freeze, and it actually seems to fit into what other programs, i.e. Outlook and Internet Explorer, do).
Attachment #8352457 - Flags: review?(florian)
*** Original post on bio 841 at 2011-06-16 09:01:03 UTC ***

Comment on attachment 8352457 [details] [diff] [review] (bio-attmnt 715)
v1.0

>diff --git a/instantbird/components/ibStatusCommandLineHandler.js b/instantbird/components/ibStatusCommandLineHandler.js

>-    let requestedStatus;
>     let statusParam = cmdLine.getArgument(statusIndex + 1).toLowerCase();
[...]
>+    let requestedStatus = Status.toFlag(statusParam);
> 
>     // Remove the arguments since they've been handled.
>     cmdLine.removeArguments(statusIndex, statusIndex + 1);
> 
>     // We're keeping the old status message here.
>     Services.core.setStatus(requestedStatus,
>                             Services.core.currentStatusMessage);

The requestedStatus variable doesn't seem needed anymore, the Status.toFlag call could just be inlined in the setStatus call.


>diff --git a/instantbird/locales/en-US/chrome/instantbird/winjumplist.properties b/instantbird/locales/en-US/chrome/instantbird/winjumplist.properties
>deleted file mode 100644
>--- a/instantbird/locales/en-US/chrome/instantbird/winjumplist.properties
>+++ /dev/null
>@@ -1,11 +0,0 @@
>-# LOCALIZATION NOTE:
>-#   These are the labels that show up for the Jump List items.
>-available.label=Available
>-away.label=Away
>-offline.label=Offline
>-
>-# LOCALIZATION NOTE:
>-#   These are descriptions of what each Jump List item will do.
>-available.tooltip=Set status to available.
>-away.tooltip=Set status to away.
>-offline.tooltip=Set status to offline.

Aren't we breaking the string freeze anyway if we remove a localized file? I don't know if this matters. If it does, then we should remove this file after the release.
Attached patch v1.1 (obsolete) — Splinter Review
*** Original post on bio 841 as attmnt 718 at 2011-06-16 12:31:00 UTC ***

So hopefully this applies properly. I added a note to the top of the file saying it's unused. Note that this doesn't remove the file from being included (from jar.mn), I seem to have missed that in my original patch and now I'm unsure whether it should be there or not.
Attachment #8352460 - Flags: review?(florian)
Comment on attachment 8352457 [details] [diff] [review]
v1.0

*** Original change on bio 841 attmnt 715 at 2011-06-16 12:31:44 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352457 - Attachment is obsolete: true
Attachment #8352457 - Flags: review?(florian)
Attached patch v1.2Splinter Review
*** Original post on bio 841 as attmnt 719 at 2011-06-16 16:27:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Comment on attachment 8352461 [details] [diff] [review]
v1.2

*** Original change on bio 841 attmnt 719 at 2011-06-16 23:41:01 UTC ***

Verified that this works fine.
Attachment #8352461 - Flags: review+
Comment on attachment 8352460 [details] [diff] [review]
v1.1

*** Original change on bio 841 attmnt 718 at 2011-06-16 23:41:21 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352460 - Attachment is obsolete: true
Attachment #8352460 - Flags: review?(florian)
*** Original post on bio 841 at 2011-06-17 00:02:05 UTC ***

https://hg.instantbird.org/instantbird/rev/8fbb05440f2e
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
You need to log in before you can comment on or make changes to this bug.