Last Comment Bug 420469 - Remove superfluous access key in Account settings -> Server settings
: Remove superfluous access key in Account settings -> Server settings
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Account Manager (show other bugs)
: Trunk
: All All
: -- minor (vote)
: Thunderbird 3
Assigned To: Hendrik Maryns
:
:
Mentors:
http://mxr.mozilla.org/mozilla/source...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-01 08:15 PST by Hendrik Maryns
Modified: 2008-05-29 09:48 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch removing the items in TB (2.57 KB, patch)
2008-03-01 08:15 PST, Hendrik Maryns
no flags Details | Diff | Splinter Review
just setting them blank (2.61 KB, patch)
2008-03-01 09:32 PST, Hendrik Maryns
no flags Details | Diff | Splinter Review
remove from .dtd and .xul (8.79 KB, patch)
2008-03-05 03:10 PST, Hendrik Maryns
mozilla: review+
neil: superreview+
Details | Diff | Splinter Review

Description Hendrik Maryns 2008-03-01 08:15:27 PST
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.
Comment 1 Hendrik Maryns 2008-03-01 08:17:56 PST
Additionally, the key newsrcFilePath.accesskey probably can be removed as well, for similar reasons.
Comment 2 Hendrik Maryns 2008-03-01 09:32:01 PST
Created attachment 306753 [details] [diff] [review]
just setting them blank

Ok, silly me, one cannot just remove them, instead, I made them blank.
Comment 3 David :Bienvenu 2008-03-04 15:34:23 PST
seems like the access keys should be removed from the xul as well, instead of just making them blank.
Comment 4 Hendrik Maryns 2008-03-05 00:52:52 PST
(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.)
Comment 5 Hendrik Maryns 2008-03-05 03:10:21 PST
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.
Comment 6 Marco Zehe (:MarcoZ) 2008-05-20 04:33:19 PDT
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.
Comment 7 Mark Banner (:standard8) 2008-05-23 04:11:04 PDT
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
Comment 8 Hendrik Maryns 2008-05-25 12:20:42 PDT
(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 Robert Kaiser 2008-05-28 11:42:38 PDT
(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.
Comment 10 Hendrik Maryns 2008-05-29 02:32:47 PDT
Well then.

Note You need to log in before you can comment on or make changes to this bug.