Instructions in account wizard not platform dependent

RESOLVED FIXED in Thunderbird 12.0

Status

MailNews Core
Account Manager
--
trivial
RESOLVED FIXED
11 years ago
5 years ago

People

(Reporter: Hendrik Maryns, Assigned: aceman)

Tracking

({uiwanted, ux-consistency})

Trunk
Thunderbird 12.0
uiwanted, ux-consistency

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

11.73 KB, patch
Ian Neal
: review+
standard8
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
haveSmtp1.suffix, haveSmtp2.suffix and haveSmtp1.suffix mention Account Settings in the Tools menu explicitly.  This should be changed to something like &pref.menuPath; but for the account settings, because thay are elsewhere on Mac and Linux.
(Reporter)

Comment 1

11 years ago
Forgot to tell: they are in AccountWizard.dtd under mail locale, ...

Updated

11 years ago
Severity: normal → minor

Updated

9 years ago
Assignee: mscott → nobody
(Assignee)

Comment 2

5 years ago
Good bug, Wayne :)

But I am not sure at which point in the Wizard this text is shown.
And whether this problem still exists.
Assignee: nobody → acelists
Keywords: uiwanted, ux-consistency
roland might be able to check if it still exists, having both Mac and windows.
(Assignee)

Comment 4

5 years ago
I have Windows and Linux, I could check it, but don't know where to look. Where in the Account wizard does this appear.
(Reporter)

Comment 5

5 years ago
(In reply to :aceman from comment #2)
> Good bug, Wayne :)

I’m not Wayne.

> But I am not sure at which point in the Wizard this text is shown.
> And whether this problem still exists.

I searched myself and didn’t find it either.  I stumbled over this during l10n.  So they might be unused strings.  How to check that?
(Assignee)

Comment 6

5 years ago
Maybe grep the whole comm-central source tree for occurrence of the haveSmtp*.suffix identifiers. Maybe 'hg grep' would be even better to find them in history, maybe their usage was removed but the strings remained.
THe wizard changed a lot recently so this bug might now be INVALID.
(In reply to Ludovic Hirlimann [:Usul] from comment #7)
> THe wizard changed a lot recently so this bug might now be INVALID.

iirc, the old wizard code still exists. but I could be wrong. but even if it does exist, it very well might not be worth fixing.
(Assignee)

Comment 9

5 years ago
If it exists, shouldn't it be completely removed?

Comment 10

5 years ago
The old wizard still exists:
- in thunderbird, for news (nntp) and movemail accounts
- in seamonkey, for everything (until they migrate to the new wizard)

I agree with WONTFIX due to irrelevance/insignificance, though.

Comment 11

5 years ago
Since these are SMTP strings, they're probably not used in Thunderbird anymore. NNTP and Movemail don't seem to use these, and RSS doesn't need SMTP at all.
(Reporter)

Comment 12

5 years ago
(In reply to :aceman from comment #6)
> Maybe grep the whole comm-central source tree for occurrence of the
> haveSmtp*.suffix identifiers. Maybe 'hg grep' would be even better to find
> them in history, maybe their usage was removed but the strings remained.

I haven’t found anything with MXR.  A developer should do hg grep, I do not have an entire checkout.  Seems like they can be removed.
(Assignee)

Comment 13

5 years ago
I do not see haveSmtp1.suffix and haveSmtp2.suffix defined anywhere in current trunk.

There is haveSmtp1.suffix2 and haveSmtp2.suffix2 but that is still used in base/prefs/content/AccountWizard.xul . 

I don't see anything to be done.
(Assignee)

Comment 14

5 years ago
(In reply to Hendrik Maryns from comment #5)
> I searched myself and didn’t find it either.  I stumbled over this during
> l10n.  So they might be unused strings.  How to check that?

Where do you see those strings defined?
(Reporter)

Comment 15

5 years ago
(In reply to :aceman from comment #14)
> (In reply to Hendrik Maryns from comment #5)
> > I searched myself and didn’t find it either.  I stumbled over this during
> > l10n.  So they might be unused strings.  How to check that?
> 
> Where do you see those strings defined?

http://mxr.mozilla.org/comm-central/source/mail/locales/en-US/chrome/messenger/AccountWizard.dtd#65
It seems to be OK here.

http://mxr.mozilla.org/comm-central/source/suite/locales/en-US/chrome/mailnews/pref/AccountWizard.dtd#69 and #93
This is where the problem is.

I suggest changing the second file’s strings to match with the first file’s.

Or rather merge the files?
(Assignee)

Comment 16

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

The strings can't be the same as the Seamonkey menu item really is labelled differently. But I suggest just removing the reference to the Edit menu and not create separate strings due to low significance (comment 10).
Attachment #586533 - Flags: review?(iann_bugzilla)
(Assignee)

Updated

5 years ago
Assignee: acelists → nobody
Severity: minor → trivial
Status: NEW → ASSIGNED
Component: Account Manager → MailNews: Account Configuration
OS: Linux → All
Product: Thunderbird → SeaMonkey
QA Contact: account-manager → mailnews-account
Hardware: x86 → All
Version: 1.5 → Trunk
(Assignee)

Comment 17

5 years ago
(In reply to Hendrik Maryns from comment #15)
> http://mxr.mozilla.org/comm-central/source/mail/locales/en-US/chrome/
> messenger/AccountWizard.dtd#65
> It seems to be OK here.
> 
> http://mxr.mozilla.org/comm-central/source/suite/locales/en-US/chrome/
> mailnews/pref/AccountWizard.dtd#69 and #93
> This is where the problem is.

Thanks for the links. I was ignoring the '*.suffix2' strings as you didn't mention them in the original description. Maybe a typo as you have haveSmtp1.suffix there twice.

Comment 18

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

As we are changing localisable strings we should be changing the ENTITY id, so from suffix2 to suffix3. This will mean changing the xul and the mail locale files too.
Seeing you need to touch the mail locale stuff anyway, another option would be to, whilst you are at it, create a new ENTITY to contain the text that is being repeated e.g.
<!ENTITY modifyOutgoing.suffix "You can modify outgoing server settings by choosing Mail &amp; Newsgroups Account Settings from the menu.">
and change the xul to use the various haveSmtp(x).suffix3 along with this new suffix.
r- for the moment
Attachment #586533 - Flags: review?(iann_bugzilla) → review-
(Assignee)

Comment 19

5 years ago
Created attachment 586821 [details] [diff] [review]
patch v2
Assignee: nobody → acelists
Attachment #586533 - Attachment is obsolete: true
Attachment #586821 - Flags: review?(iann_bugzilla)
(Reporter)

Comment 20

5 years ago
(In reply to Ian Neal from comment #18)
> Comment on attachment 586533 [details] [diff] [review]
> patch
> 
> As we are changing localisable strings we should be changing the ENTITY id,
> so from suffix2 to suffix3. 

Isn’t this, like, violating the very reason why externalised strings where introduced anyway?  So that you can change them without having to change your code also?  Seems quite absurd to me if this is policy.
(Assignee)

Comment 21

5 years ago
That is true, if Mozilla would be English only, it would work as you say. But when a string changes, the ID must be changed so that translators notice something has changed there. Yes it is totally bad and I can't believe myself the translation tools working on mozilla are that stupid and can't see a string change (in the English template) without changed ID. But this is what I am told everywhere, that this is the current state of affairs.

Comment 22

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

>+++ b/mail/locales/en-US/chrome/messenger/AccountWizard.dtd
>+<!ENTITY modifyOutgoing.suffix "You can change outgoing servers in the account settings.">
Might be worth adding a note about how this is used.

>+++ b/suite/locales/en-US/chrome/mailnews/pref/AccountWizard.dtd
>+<!ENTITY modifyOutgoing.suffix "You can modify outgoing server settings by choosing Mail &amp; Newsgroups Account Settings from the menu.">
Might be worth adding a note about how this is used.
I would prefer something like "You can modify outgoing server settings from within Mail &amp; Newsgroups Account Settings."

r=me but you will need an additional/super review from the TB side.
Attachment #586821 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 23

5 years ago
Created attachment 587441 [details] [diff] [review]
patch v3
Attachment #586821 - Attachment is obsolete: true
Attachment #587441 - Flags: superreview?(bwinton)
Attachment #587441 - Flags: review?(iann_bugzilla)
Comment on attachment 587441 [details] [diff] [review]
patch v3

I'm not actually that super, so I'm redirecting to Standard8…  ;)
Attachment #587441 - Flags: superreview?(bwinton) → superreview?(mbanner)

Updated

5 years ago
Attachment #587441 - Flags: review?(iann_bugzilla) → review+
Component: MailNews: Account Configuration → Account Manager
Product: SeaMonkey → MailNews Core
QA Contact: mailnews-account → account-manager
Comment on attachment 587441 [details] [diff] [review]
patch v3

Blake might not be that super, but he should probably take a quick look at the strings ;-)
Attachment #587441 - Flags: ui-review?(bwinton)
Comment on attachment 587441 [details] [diff] [review]
patch v3

Actually, Blake doesn't need to look at the strings, because we don't display these on Thunderbird. So just sr=me.
Attachment #587441 - Flags: ui-review?(bwinton)
Attachment #587441 - Flags: superreview?(mbanner)
Attachment #587441 - Flags: superreview+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/0989a2c506db
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 12.0
You need to log in before you can comment on or make changes to this bug.