Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Enhance wording of compacting option

RESOLVED FIXED in Thunderbird 16.0

Status

Thunderbird
Preferences
--
minor
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ulf Zibis, Assigned: aceman)

Tracking

({polish})

Trunk
Thunderbird 16.0
polish
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

10.28 KB, patch
mconley
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:12.0) Gecko/20100101 Firefox/12.0
Build ID: 20120420145725

Steps to reproduce:

I worried about the wording in the options settings pane.
At least the german translation does not clearly describe the fact, that the threshold is meant as sum over all folders, because in german the plural form for 'folder' is the same than the singular form. 


Actual results:

The german translation in options of:
"Compact folder, if that saves more space than x MB"
IMO is ambiguous. It's not clear, if
... that is meant as sum over all folders rather than space per folder.
... only that big folders are compacted at all.



Expected results:

If you would add the words 'all' and 'in sum', also the german (and maybe others too) translation would be unambiguously clear:
"Compact all folders, if that saves more space in sum than x MB"
(Reporter)

Updated

5 years ago
Depends on: 711765
OS: Windows XP → All
Hardware: x86 → All
See Also: → bug 711765
(Assignee)

Comment 1

5 years ago
If the English text is in plural, the translator should know how to properly express that in his language. But maybe he didn't understand what it means.
I would agree with making the text more precise also in English, because it can be understood in different ways, as you say.
Bwinton?
Assignee: nobody → acelists
(Assignee)

Updated

5 years ago
Severity: normal → minor
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: polish, uiwanted
(Assignee)

Comment 2

5 years ago
Seamonkey has the same string. Ian, should I update it there too?
(Assignee)

Comment 3

5 years ago
Created attachment 631676 [details] [diff] [review]
patch

Bwinton: this is the wording proposal. And it also makes the textbox disabled when compacting is disabled. Other fields in the prefs do it too.

Ian: this is the wording proposal. Would you like it in Seamonkey?
Attachment #631676 - Flags: ui-review?(bwinton)
Attachment #631676 - Flags: feedback?(iann_bugzilla)

Comment 4

5 years ago
Comment on attachment 631676 [details] [diff] [review]
patch

>+++ b/mail/components/preferences/advanced.xul
>@@ -74,18 +74,21 @@
>                   name="mailnews.mark_message_read.delay" type="bool"
>                   onchange="gAdvancedPane.updateMarkAsReadTextbox(this.value);"/>
>       <preference id="mailnews.mark_message_read.delay.interval"
>                   name="mailnews.mark_message_read.delay.interval" type="int"/>
>       <preference id="mail.openMessageBehavior" name="mail.openMessageBehavior" type="int"/>
>       <preference id="mail.close_message_window.on_delete" 
>                   name="mail.close_message_window.on_delete" type="bool"/>
>       <!-- Network tab -->
>+      <preference id="mail.prompt_purge_threshhold"
>+                  name="mail.prompt_purge_threshhold"
>+                  type="bool"/>
The pattern in this file seems to have the type on the same line as the name.
>+      <preference id="mail.purge_threshhold_mb"
>+                  name="mail.purge_threshhold_mb" type="int"/>

>           <groupbox>
>-            <caption label="&Diskspace;"/>
>+            <caption label="&diskspace;"/>
Do you really need to change the entity?

>+                <checkbox id="offlineCompactFolder"
>+                          label="&offlineCompactFolders.label;"
>+                          accesskey="&offlineCompactFolders.accesskey;"
>+                          oncommand="gAdvancedPane.updateCompactOptions(this.checked);"
You could add an onchange to the preference instead (SM does, but not sure if there is an advantage).
>                           preference="mail.prompt_purge_threshhold"/>
Does the checkbox need an aria-labelledby? (SM does, though one entry is incorrect for SM on this and the textbox)

Yes, SM would benefit from this clarity too.
Attachment #631676 - Flags: feedback?(iann_bugzilla) → feedback+
(Assignee)

Comment 5

5 years ago
(In reply to Ian Neal from comment #4)
> >           <groupbox>
> >-            <caption label="&Diskspace;"/>
> >+            <caption label="&diskspace;"/>
> Do you really need to change the entity?
No, I just thought to clean up the anomalous uppercase. I can drop this.

> >+                <checkbox id="offlineCompactFolder"
> >+                          label="&offlineCompactFolders.label;"
> >+                          accesskey="&offlineCompactFolders.accesskey;"
> >+                          oncommand="gAdvancedPane.updateCompactOptions(this.checked);"
> You could add an onchange to the preference instead (SM does, but not sure
> if there is an advantage).
This file uses both approaches. I tried the <preference onchange=> but it didn't know this.checked. I would have to play with this.value. Depends on what values it has. I'll try.

> >                           preference="mail.prompt_purge_threshhold"/>
> Does the checkbox need an aria-labelledby? (SM does, though one entry is
> incorrect for SM on this and the textbox)
No idea but I can try to copy/fix the SM definition.

> Yes, SM would benefit from this clarity too.
OK, I'll try to apply the string to SM too.
(Assignee)

Comment 6

5 years ago
But first wait for bwinton's word on the wording.
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
Comment on attachment 631676 [details] [diff] [review]
patch

Yeah, sure, I'll say ui-r=me for this wording.
Attachment #631676 - Flags: ui-review?(bwinton) → ui-review+
(Assignee)

Updated

5 years ago
Attachment #631676 - Flags: review?(iann_bugzilla)

Comment 8

5 years ago
Comment on attachment 631676 [details] [diff] [review]
patch

Wrong patch? no SeaMonkey parts?
Attachment #631676 - Flags: review?(iann_bugzilla) → review-
(Assignee)

Comment 9

5 years ago
Created attachment 632003 [details] [diff] [review]
patch v2

Sorry. That was just the original version of the patch that waited on bwinton. I should not r? you on that. Here is the new version with SM parts added and your nits.
Attachment #631676 - Attachment is obsolete: true
Attachment #632003 - Flags: review?(iann_bugzilla)

Comment 10

5 years ago
Comment on attachment 632003 [details] [diff] [review]
patch v2

>+++ b/suite/mailnews/prefs/pref-offline.xul
>         <textbox id="offlineCompactFolderMin"
>                  type="number"
>                  size="5"
This needs changing to size="4" (not sure why it is 5 to start with) for the new text to fit within the page.
>                  min="1"
>                  max="2048"
>                  increment="10"
>                  value="20"
>                  preference="mail.purge_threshhold_mb"
>-                 aria-labelledby="offlineCompactFolder offlineCompactFolderMin kbLabel"/>

r=me with that fixed.
Attachment #632003 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 11

5 years ago
Created attachment 632027 [details] [diff] [review]
patch v3

Thanks, reduced the size in TB too. It still shows space for 5 digits, but that seems to be a general problem with a textbox.
Attachment #632003 - Attachment is obsolete: true
Attachment #632027 - Flags: review?(mconley)
(Assignee)

Updated

5 years ago
No longer depends on: 711765
Keywords: uiwanted
Comment on attachment 632027 [details] [diff] [review]
patch v3

Review of attachment 632027 [details] [diff] [review]:
-----------------------------------------------------------------

This mostly looks good - just one point.

::: mail/components/preferences/advanced.xul
@@ +315,5 @@
>              <hbox align="center">
> +                <checkbox id="offlineCompactFolder"
> +                          label="&offlineCompactFolders.label;"
> +                          accesskey="&offlineCompactFolders.accesskey;"
> +                          oncommand="gAdvancedPane.updateCompactOptions(this.checked);"

How hard would it be to use the command pattern here, instead of oncommand? The less we introduce usage of oncommand, the better.
Comment on attachment 632027 [details] [diff] [review]
patch v3

Whoops, forgot to set the r- flag.
Attachment #632027 - Flags: review?(mconley) → review-
(Assignee)

Comment 14

5 years ago
There is no other usage of command in the file (maybe whole preferences dialog). So it would be hard for me to start the usage and have nothing to copy :) But let's try it.
(Assignee)

Comment 15

5 years ago
Created attachment 635431 [details] [diff] [review]
patch v4

Is this enough?
Attachment #632027 - Attachment is obsolete: true
Attachment #635431 - Flags: review?(mconley)
Comment on attachment 635431 [details] [diff] [review]
patch v4

Review of attachment 635431 [details] [diff] [review]:
-----------------------------------------------------------------

I'm good with this. Thanks, aceman.
Attachment #635431 - Flags: review?(mconley) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/54bf0eb08ceb
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 16.0
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.