Closed Bug 953901 Opened 10 years ago Closed 10 years ago

Accesskeys on preferences window

Categories

(Instantbird Graveyard :: Preferences, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: benediktp, Assigned: benediktp)

References

Details

Attachments

(1 file, 4 obsolete files)

*** Original post on bio 462 at 2010-07-31 09:39:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
*** Original post on bio 462 as attmnt 321 at 2010-07-31 09:39:00 UTC ***

There are several flaws with the accesskeys on the preferences window. 

I added missing accesskeys on the themes pane (both tabs) and changed some others on other panes. I also had to change an existing and working one of the Mac accesskeys (the one for "show dock icon") to free a reasonable letter for another accesskey.
Attachment #8352062 - Flags: review?(florian)
Assignee: nobody → benediktp
*** Original post on bio 462 at 2010-08-01 16:37:18 UTC ***

Changing the accesskey for "Show Header" from "S" to "H" would make more sense. I'll replace the patch later, I only need to check if I find even more things that need a change.
*** Original post on bio 462 as attmnt 328 at 2010-08-09 17:54:00 UTC ***

Fixed the "Show Header" accesskey to "H" only.
Comment on attachment 8352062 [details] [diff] [review]
Corrects and adds accesskeys on the preferences

*** Original change on bio 462 attmnt 321 at 2010-08-09 17:54:29 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352062 - Attachment is obsolete: true
Attachment #8352062 - Flags: review?(florian)
Comment on attachment 8352069 [details] [diff] [review]
Change accesskey for "Show Header"

*** Original change on bio 462 attmnt 328 at 2010-08-09 17:55:24 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352069 - Flags: review?(florian)
Comment on attachment 8352069 [details] [diff] [review]
Change accesskey for "Show Header"

*** Original change on bio 462 attmnt 328 at 2010-08-09 22:53:11 UTC ***

>diff --git a/instantbird/locales/en-US/chrome/instantbird/preferences/main.dtd b/instantbird/locales/en-US/chrome/instantbird/preferences/main.dtd
>--- a/instantbird/locales/en-US/chrome/instantbird/preferences/main.dtd
>+++ b/instantbird/locales/en-US/chrome/instantbird/preferences/main.dtd

>@@ -12,9 +12,9 @@
> 
> <!ENTITY newMessage.label              "When receiving a new message">
> <!ENTITY getAttention.label            "Flash the taskbar item">
> <!ENTITY getAttention.accesskey        "F">
> <!ENTITY getAttentionMac.label         "Animate dock icon">
>-<!ENTITY getAttentionMac.accesskey     "A">
>+<!ENTITY getAttentionMac.accesskey     "D">
s/D/d/
I'm not sure if it's required or not, but other accesskeys in the same file (at least) match the case of the letter in the label, so I think we should do it here too for consistency.

>diff --git a/instantbird/content/preferences/themes.xul b/instantbird/content/preferences/themes.xul
>--- a/instantbird/content/preferences/themes.xul
>+++ b/instantbird/content/preferences/themes.xul
>@@ -75,7 +75,8 @@
>       <tabpanels flex="1">
>         <tabpanel orient="vertical" flex="1">
>           <hbox align="baseline">
>-            <label value="&messageStyleTheme.label;" control="themename"/>
>+            <label value="&messageStyleTheme.label;" control="themename" 
>+                   accesskey="&messageStyleTheme.accesskey;" />

Coding style nit: Please remove the space before "/>".
This comment applies too for a few other places in this patch.
This space was a recommended in XHTML for backward compatibility with HTML, so that old HTML parsers read the / as an attribute.
XUL is parsed as XML, so this space is not needed. Thus, it's just a matter of coding style :-).

r=me with those two nits fixed. I'm assuming you have tested this.

Currently the strings for 0.3a1pre and 0.2 are still the same, which is used/abused by translators. We will need to figure out a good way for them to decide if they work on the 0.2 branch or on the trunk soon, but I may delay landing this patch until we definitely need to break string compatibility between 0.2 and 0.3a1pre (this will happen when we move to Mozilla 2.0 or when we upgrade libpurple to 2.7.*. Both are likely to be done by the end of the month).
Attachment #8352069 - Flags: review?(florian) → review+
Attached patch Added changes required for r+ (obsolete) — Splinter Review
*** Original post on bio 462 as attmnt 329 at 2010-08-10 10:26:00 UTC ***

I added the changes. The strings are tested on "running copy" of Instantbird and were moved to the repository after I found them working. I think I worked thoroughly.

I hope this patch will apply, I downloaded the old one and replaced the changes there (and there seemed to be an encoding problem on the webpage (ellipsis character was broken, replaced it then))
Comment on attachment 8352069 [details] [diff] [review]
Change accesskey for "Show Header"

*** Original change on bio 462 attmnt 328 at 2010-08-10 10:26:38 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352069 - Attachment is obsolete: true
*** Original post on bio 462 as attmnt 332 at 2010-08-22 14:52:00 UTC ***

Changed label of button to upper case "Show Log Folder". Matchs better with other button labels now.
Comment on attachment 8352070 [details] [diff] [review]
Added changes required for r+

*** Original change on bio 462 attmnt 329 at 2010-08-22 14:52:20 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352070 - Attachment is obsolete: true
*** Original post on bio 462 as attmnt 350 at 2010-09-02 18:32:00 UTC ***

Changed "Manage search engines" to uppercase and added ellipsis similiar to other buttons that open dialogs or windows.
Attachment #8352091 - Flags: review?(florian)
Comment on attachment 8352073 [details] [diff] [review]
Same as r+ patch with additional label change

*** Original change on bio 462 attmnt 332 at 2010-09-02 18:32:19 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352073 - Attachment is obsolete: true
Status: NEW → ASSIGNED
OS: Windows XP → All
Hardware: x86 → All
Comment on attachment 8352091 [details] [diff] [review]
Same as before, additional change on button label

*** Original change on bio 462 attmnt 350 at 2010-09-14 13:30:56 UTC ***

Looks good. Thanks for taking care of this!
Attachment #8352091 - Flags: review?(florian) → review+
*** Original post on bio 462 at 2010-09-14 18:47:04 UTC ***

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