Note: There are a few cases of duplicates in user autocompletion which are being worked on.

MOZ_MOVEMAIL ifdef not working for account central page

RESOLVED FIXED in Thunderbird 25.0

Status

Thunderbird
Build Config
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: alta88, Assigned: alta88)

Tracking

Trunk
Thunderbird 25.0
x86_64
Linux

Thunderbird Tracking Flags

(thunderbird24+ fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.92 KB, patch
jcranmer
: review+
standard8
: review+
Details | Diff | Splinter Review
(Assignee)

Description

4 years ago
the ifdef here
http://mxr.mozilla.org/comm-central/source/mailnews/base/content/msgAccountCentral.xul#193

doesn't result in the Movemail label appearing in linux.

Comment 1

4 years ago
What about HAVE_MOVEMAIL ?
(Assignee)

Comment 2

4 years ago
there's no Makefile.in in mailnews/base/content, hence no such declaration.  since build config is in flux, i don't know what the proper solution is, ie a Makefile.in or putting this in a moz.build.
Flags: needinfo?(mbanner)
(In reply to alta88 from comment #2)
> there's no Makefile.in in mailnews/base/content, hence no such declaration. 
> since build config is in flux, i don't know what the proper solution is, ie
> a Makefile.in or putting this in a moz.build.

The msgAccountCentral.xul gets preprocessed from mailnews/' jar.mn, so the DEFINES declaration in mailnews/Makefile.in is the appropriate place.

Now, that said, adding things to DEFINES in makefiles is still Bad Practice™, and given the number of places we ad-hoc add it to defines, plopping an AC_DEFINE in the configure.in files would work much better IMHO.
Flags: needinfo?(mbanner)
(Assignee)

Comment 4

4 years ago
well, it's all over the place and what should be done is the Right Thing™ but that often leads straight to No Good Deed...™

since there is no mailnews/configure.in 
1. should one be created, for this
2. does mailnews/ pick up root configure.in
3. does mail/ pick up root, making it unnecessary in its configure.in

you have a better idea of where build config is going, so the most effective thing to do is sketch out specifically how best to clean up this particular definition.
(In reply to alta88 from comment #4)
> well, it's all over the place and what should be done is the Right Thing™
> but that often leads straight to No Good Deed...™
> 
> since there is no mailnews/configure.in 
> 1. should one be created, for this
> 2. does mailnews/ pick up root configure.in
> 3. does mail/ pick up root, making it unnecessary in its configure.in

4. Put the definitions in mail/configure.in and suite/configure.in, around the other AC_SUBST(MOZ_MOVEMAIL) lines (indeed, suite/configure.in and mail/configure.in are nearly full clones of each other).
(Assignee)

Comment 6

4 years ago
Created attachment 771343 [details] [diff] [review]
patch


this builds/works; if no issues a suite r will be requested.
Assignee: nobody → alta88
Attachment #771343 - Flags: review?(Pidgeot18)
Comment on attachment 771343 [details] [diff] [review]
patch

Review of attachment 771343 [details] [diff] [review]:
-----------------------------------------------------------------

The configure.in files seem wrong; HAVE_MOVEMAIL=1 should also be defined beforehand.
Attachment #771343 - Flags: review?(Pidgeot18) → review-
(Assignee)

Comment 8

4 years ago
Created attachment 771880 [details] [diff] [review]
patch2
Attachment #771880 - Flags: review?(Pidgeot18)
Attachment #771880 - Flags: review?(Pidgeot18) → review+
(Assignee)

Updated

4 years ago
Attachment #771880 - Flags: review?(neil)
Comment on attachment 771880 [details] [diff] [review]
patch2

You want a build config review for this, and as it happens I'm a peer of the SM module, so r=me.
Attachment #771880 - Flags: review?(neil) → review+
(Assignee)

Comment 10

4 years ago
this should follow bug 529131.
tracking-thunderbird24: --- → ?
Keywords: checkin-needed
Attachment #771343 - Attachment is obsolete: true
https://hg.mozilla.org/comm-central/rev/7d8d252b7056
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 25.0
Blocks: 529131
tracking-thunderbird24: ? → +
Comment on attachment 771880 [details] [diff] [review]
patch2

[Triage Comment]
Will take to aurora, as bug 529131 landed there
Attachment #771880 - Flags: approval-comm-aurora+
https://hg.mozilla.org/releases/comm-aurora/rev/b2ae1dae3739
status-thunderbird24: --- → fixed
You need to log in before you can comment on or make changes to this bug.