346306 - Instructions in account wizard not platform dependent

Status: RESOLVED FIXED

(MailNews Core :: Account Manager, defect)

Description

[Hendrik Maryns]

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.

Comment 1

[Hendrik Maryns]

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

Comment 2

[:aceman]

Good bug, Wayne :)

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

Comment 3

[Wayne Mery (:wsmwk)]

roland might be able to check if it still exists, having both Mac and windows.

Comment 4

[:aceman]

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

Comment 5

[Hendrik Maryns]

(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?

Comment 6

[:aceman]

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.

Comment 7

[Ludovic Hirlimann [:Usul]]

THe wizard changed a lot recently so this bug might now be INVALID.

Comment 8

[Wayne Mery (:wsmwk)]

(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.

Comment 9

[:aceman]

If it exists, shouldn't it be completely removed?

Comment 10

[Ben Bucksch (:BenB)]

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

[Jim Porter (:squib)]

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.

Comment 12

[Hendrik Maryns]

(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.

Comment 13

[:aceman]

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.

Comment 14

[:aceman]

(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?

Comment 15

[Hendrik Maryns]

(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?

Comment 16

[:aceman]

Attachment: 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).

Comment 17

[:aceman]

(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

[Ian Neal]

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

Comment 19

[:aceman]

Attachment: patch v2

Comment 20

[Hendrik Maryns]

(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.

Comment 21

[:aceman]

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

[Ian Neal]

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.

Comment 23

[:aceman]

Attachment: patch v3

Comment 24

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 587441 [details] [diff] [review]
patch v3

I'm not actually that super, so I'm redirecting to Standard8…  ;)

Comment 25

[Mark Banner (:standard8)]

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 ;-)

Comment 26

[Mark Banner (:standard8)]

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.

Comment 27

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/0989a2c506db