Remove superfluous access key in Account settings -> Server settings

RESOLVED FIXED in Thunderbird 3

Status

Thunderbird
Account Manager
--
minor
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Hendrik Maryns, Assigned: Hendrik Maryns)

Tracking

Trunk
Thunderbird 3

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

9 years ago
Created attachment 306743 [details] [diff] [review]
patch removing the items in TB

The access key localPath.accesskey in am-server-top.dtd and am-serverwithnoidentities.dtd (both in mail/.../messenger and in suite/mailnews/.../pref) is superfluous: it directs one to the text field, but this text field is not editable.
Since there are really a lot of options on this window (too much?), it gets very hard to find suitable letters for the access key in localizations (English has double access keys there as well).  Therefore, it would be best just to remove it.
Attachment #306743 - Flags: review?(bienvenu)
(Assignee)

Comment 1

9 years ago
Additionally, the key newsrcFilePath.accesskey probably can be removed as well, for similar reasons.
Status: NEW → ASSIGNED
(Assignee)

Comment 2

9 years ago
Created attachment 306753 [details] [diff] [review]
just setting them blank

Ok, silly me, one cannot just remove them, instead, I made them blank.
Attachment #306743 - Attachment is obsolete: true
Attachment #306753 - Flags: review?
Attachment #306743 - Flags: review?(bienvenu)
(Assignee)

Updated

9 years ago
Attachment #306753 - Flags: review? → review?(bienvenu)
Assignee: nobody → hendrik.maryns
Status: ASSIGNED → NEW

Comment 3

9 years ago
seems like the access keys should be removed from the xul as well, instead of just making them blank.
(Assignee)

Comment 4

9 years ago
(In reply to comment #3)
> seems like the access keys should be removed from the xul as well, instead of
> just making them blank.

So you do agree that they should be removed in the first place?  (Before I start putting more time in this.)
(Assignee)

Comment 5

9 years ago
Created attachment 307441 [details] [diff] [review]
remove from .dtd and .xul

As you suggested, removing them altogether.  Also in suite, since they share the mailnews code.  Tested, works fine.
Attachment #306753 - Attachment is obsolete: true
Attachment #306753 - Flags: review?(bienvenu)
Attachment #307441 - Flags: review?
(Assignee)

Updated

9 years ago
Attachment #307441 - Flags: review? → review?(bienvenu)

Updated

9 years ago
Attachment #307441 - Flags: superreview?(neil)
Attachment #307441 - Flags: review?(bienvenu)
Attachment #307441 - Flags: review+

Comment 6

9 years ago
To answer Neil's question on IRC: Yes, it makes the most sense to remove them from readonly textboxes (more so than other controls) since these will most likely only be used to copy & paste the path. OK with me.

Updated

9 years ago
Attachment #307441 - Flags: superreview?(neil) → superreview+
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Patch checked in. Hendrik, please mark as fixed if there's no more changes to be done on this bug.

Checking in mail/locales/en-US/chrome/messenger/am-server-top.dtd;
/cvsroot/mozilla/mail/locales/en-US/chrome/messenger/am-server-top.dtd,v  <--  am-server-top.dtd
new revision: 1.10; previous revision: 1.9
done
Checking in mail/locales/en-US/chrome/messenger/am-serverwithnoidentities.dtd;
/cvsroot/mozilla/mail/locales/en-US/chrome/messenger/am-serverwithnoidentities.dtd,v  <--  am-serverwithnoidentities.dtd
new revision: 1.4; previous revision: 1.3
done
Checking in mailnews/base/prefs/resources/content/am-server.xul;
/cvsroot/mozilla/mailnews/base/prefs/resources/content/am-server.xul,v  <--  am-server.xul
new revision: 1.124; previous revision: 1.123
done
Checking in mailnews/base/prefs/resources/content/am-serverwithnoidentities.xul;
/cvsroot/mozilla/mailnews/base/prefs/resources/content/am-serverwithnoidentities.xul,v  <--  am-serverwithnoidentities.xul
new revision: 1.33; previous revision: 1.32
done
Checking in suite/locales/en-US/chrome/mailnews/pref/am-server-top.dtd;
/cvsroot/mozilla/suite/locales/en-US/chrome/mailnews/pref/am-server-top.dtd,v  <--  am-server-top.dtd
new revision: 1.44; previous revision: 1.43
done
Checking in suite/locales/en-US/chrome/mailnews/pref/am-serverwithnoidentities.dtd;
/cvsroot/mozilla/suite/locales/en-US/chrome/mailnews/pref/am-serverwithnoidentities.dtd,v  <--  am-serverwithnoidentities.dtd
new revision: 1.7; previous revision: 1.6
done
Keywords: checkin-needed
(Assignee)

Comment 8

9 years ago
(In reply to comment #7)
> Patch checked in. Hendrik, please mark as fixed if there's no more changes to
> be done on this bug.

Do I need to take any actions to percolate this to l10n?

Comment 9

9 years ago
(In reply to comment #8)
> Do I need to take any actions to percolate this to l10n?

No, L10n tinderboxen turn orange automatically when a checkin removes/adds string IDs.
(Assignee)

Comment 10

9 years ago
Well then.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Updated

9 years ago
Target Milestone: --- → Thunderbird 3
You need to log in before you can comment on or make changes to this bug.