Tooltips for Download toolbar buttons should update to reflect the number of items selected

RESOLVED FIXED in Camino1.5

Status

RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: froodian, Assigned: froodian)

Tracking

({fixed1.8.1.1, polish})

Trunk
Camino1.5
PowerPC
macOS
fixed1.8.1.1, polish

Details

(Whiteboard: [Good First Bug] [String changes in comment 2])

Attachments

(1 attachment, 1 obsolete attachment)

8.16 KB, patch
stuart.morgan+bugzilla
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

13 years ago
Currently we use "download(s)" and "items(s)" to talk about selected items in the download list.  Instead, the tooltips should reflect whether or not there are multiple items selected (so they'd update from "download" to "downloads" when you selected a second item).
(Assignee)

Updated

13 years ago
Whiteboard: [Good First Bug]
(Assignee)

Comment 1

13 years ago
Posted patch Patch (obsolete) — Splinter Review
Assignee: nobody → stridey
Status: NEW → ASSIGNED
Attachment #247357 - Flags: review?(bugzilla)
(Assignee)

Comment 2

13 years ago
This will need string changes such that the block for the toolbar items reads

"dlRemoveButtonTooltip" = "Remove selected item from the list";
"dlRemoveButtonTooltipPlural" = "Remove selected items from the list";
"dlRevealButtonTooltip" = "Show selected file in Finder";
"dlRevealButtonTooltipPlural" = "Show selected files in Finder";
"dlOpenButtonTooltip" = "Open selected file";
"dlOpenButtonTooltipPlural" = "Open selected files";
"dlCancelButtonTooltip" = "Cancel selected download";
"dlCancelButtonTooltipPlural" = "Cancel selected downloads";
"dlPauseButtonTooltip" = "Pause selected download";
"dlPauseButtonTooltipPlural" = "Pause selected downloads";
"dlResumeButtonTooltip" = "Resume selected download";
"dlResumeButtonTooltipPlural" = "Resume selected downloads";
"dlTrashButtonTooltip" = "Move selected file to Trash";
"dlTrashButtonTooltipPlural" = "Move selected files to Trash";
"dlCleanUpButtonTooltip" = "Remove all inactive items from the list";
Whiteboard: [Good First Bug] → [Good First Bug] [String changes in comment 2]
Comment on attachment 247357 [details] [diff] [review]
Patch

Looks reasonable. Didn't test it, though.
Attachment #247357 - Flags: review?(bugzilla) → review+
(Assignee)

Updated

13 years ago
Attachment #247357 - Flags: superreview?(stuart.morgan)

Comment 4

13 years ago
Comment on attachment 247357 [details] [diff] [review]
Patch

>+  if ([itemIdentifier isEqualToString:@"cleanupbutton"])
>+    toolTip = NSLocalizedString(@"dlCleanUpButtonTooltip", nil);
>+  else if ([itemIdentifier isEqualToString:@"removebutton"])
>+    toolTip = plural ? NSLocalizedString(@"dlRemoveButtonTooltipPlural", nil) : NSLocalizedString(@"dlRemoveButtonTooltip", nil);
>+  else if ([itemIdentifier isEqualToString:@"cancelbutton"])
>+    toolTip = plural ? NSLocalizedString(@"dlCancelButtonTooltipPlural", nil) : NSLocalizedString(@"dlCancelButtonTooltip", nil);
>+  else if ([itemIdentifier isEqualToString:@"revealbutton"])
>+    toolTip = plural ? NSLocalizedString(@"dlRevealButtonTooltipPlural", nil) : NSLocalizedString(@"dlRevealButtonTooltip", nil);
>+  else if ([itemIdentifier isEqualToString:@"openbutton"])
>+    toolTip = plural ? NSLocalizedString(@"dlOpenButtonTooltipPlural", nil) : NSLocalizedString(@"dlOpenButtonTooltip", nil);
>+  else if ([itemIdentifier isEqualToString:@"movetotrashbutton"])
>+    toolTip = plural ? NSLocalizedString(@"dlTrashButtonTooltipPlural", nil) : NSLocalizedString(@"dlTrashButtonTooltip", nil);
>+  else if ([itemIdentifier isEqualToString:@"pauseresumebutton"]) {
>+    if ([self shouldAllowResumeAction])
>+      toolTip = plural ? NSLocalizedString(@"dlResumeButtonTooltipPlural", nil) : NSLocalizedString(@"dlResumeButtonTooltip", nil);
>+    else
>+      toolTip = plural ? NSLocalizedString(@"dlPauseButtonTooltipPlural", nil) : NSLocalizedString(@"dlPauseButtonTooltip", nil);
>+  }

Move the ternary inside the NSLocalizedString call.

>+  [theItem setToolTip:toolTip];

If somehow this list and the toolbar items got out of sync at all, this would be calling setToolTip: with an undefined argument and cause potentially hard-to-debug behavior.
Attachment #247357 - Flags: superreview?(stuart.morgan) → superreview-
(Assignee)

Comment 5

13 years ago
Posted patch PatchSplinter Review
Is this nil assignment/check enough to ensure that we're never calling setToolTip when we shouldn't be?
Attachment #247357 - Attachment is obsolete: true
Attachment #247415 - Flags: superreview?(stuart.morgan)

Comment 6

13 years ago
Comment on attachment 247415 [details] [diff] [review]
Patch

>+  if (toolTip != nil)

if (toolTip)

sr=smorgan with that change.
Attachment #247415 - Flags: superreview?(stuart.morgan) → superreview+
(Assignee)

Comment 7

13 years ago
Chickened on 1.8branch and trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Keywords: fixed1.8.1.1
Resolution: --- → FIXED
Moving fixed "1.2" bugs to 1.1 where they were really fixed. Filter on CaminoFixed1.1 for bugmail purposes.
Target Milestone: Camino1.2 → Camino1.1
You need to log in before you can comment on or make changes to this bug.