327037 - [gcc4 + -O2 + i386] Newsgroup names over-abbreviated on UB Mac

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Frederic Bezies]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060213 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060213 Firefox/1.6a1

Simple. All is resumed in the summary.

Since patch for bug 317674 has landed, newsgroup name is displayed truncated.

Reproducible: Always

Steps to Reproduce:
1.Build or grab a Thunderbird / SeaMonkey after patch for bug 317674 has landed


Actual Results:  
Newsgroup name display is truncated 

Expected Results:  
Newsgroup name display not truncated.

Will attach a screenshot to show the bug which appears in both thunderbird and SM MailNews.

Comment 1

[Frederic Bezies]

Attachment: Wrong display of the newsgroup name.

I tried to find the right component. Feel free to change it if needed.

Comment 2

[Jungshik Shin]

I have no clue how that could happen, but I'll take a look. 

Comment 3

[Jungshik Shin]

I can't reproduce this problem in my debug build on Linux and Mac OS X. 

Comment 4

[Jungshik Shin]

Just adding a couple of folks who might be able to conduct more extensive tests.

Comment 5

[Mike Cowperthwaite]

Unable to reproduce with TB 1.6a1-0216, Win2K.
xref bug 318792

Comment 6

[Frederic Bezies]

I tried to do a debug build, removing static build options.

And newsgroups name are fully displayed !

Modifying summary to reflect these datas.

Comment 7

[Frederic Bezies]

Cannot see this bug in a nightly build from mozilla.org.

So, it could be related either to Gcc 4.0 and/or SDK 10.4u while making a static build :(

Modifying summary to be up-to-date with these infos.

Comment 8

[Frederic Bezies]

I tried the couple gcc 4.0.1 + SDK10.3.9 = no display problem.

So, this bug is related to gcc 4.0.1 + SDK 10.4u couple. Could it block intel based trunk build ?

Comment 9

[Jungshik Shin]

(In reply to comment #8)

> So, this bug is related to gcc 4.0.1 + SDK 10.4u couple. Could it block intel
> based trunk build ?

Probably, yes.  I wonder if there's any trunk build for intel mac (or universal binary) to test. 
 

Comment 10

[Scott MacGregor]

I'm not convinced this has anything to do with jshin's patch. I think it's a UB issue instead. Someone showed me this same problem on a 1.5.0.2 / 1.5.0.4 UB build of Thunderbird, and that doesn't have 317674 in it.

Comment 11

[David :Bienvenu]

yes, I see it in my 1.5.0.4 UB build

Comment 12

[Benjamin Smedberg]

I personally think this looks really unprofessional and should block any UB release.

Comment 13

[Marcia Knous [:marcia]]

The 1.5.0.4 UB candidate build shows this issue whether you use an existing profile or a new profile. When I run the UB on my PPC Mac (10.3.9) newsgroups display fine using the same 1.5.0.4 candidate build (using my existing profile).

Comment 14

[Marcia Knous [:marcia]]

One workaround for this is to run the UB under Rosetta, which I can confirm works with the 1.5.0.4 candidate build. 

Comment 15

[Dave Liebreich [:davel]]

what happens if you set mail.server.default.abbreviate to true, then restart tb?  What is the default value on UB vs PPC?

Comment 16

[Benjamin Smedberg]

the default is "true"... when I set it to "false" the newsgroups are displayed correctly (not abbreviated of course).

Comment 17

[Daniel Veditz [:dveditz]]

This has nothing to do with bug 317674, that bug has not landed on the 1.8.0 branch as Scott said in comment 10. This is only seen on Universal Binary macs running native on intel

Here's the abbreviation code, dunno why intel would make any difference:
http://lxr.mozilla.org/mozilla/source/mailnews/news/src/nsNewsFolder.cpp#651

Comment 18

[Mark Mentovai]

Based on early comments in this bug, it doesn't appear to be a CPU/architecture-dependent bug, but a compiler/SDK-dependent one.  The comments say that gcc 4 with the 10.4u SDK shows the bug, but gcc 4 with the 10.3.9 SDK does not.  If this is true, then the bug is "only seen" on universals running natively on x86 because that's the only official release configuration built with that compiler and SDK.  If all of the comments are true, I'd also expect a gcc 3.3 ppc build with the 10.4u SDK to show the bug.  Can anyone confirm this?

The comments also indicate that this is a problem in static-optimize builds but not dynamic-debug builds.  If someone can find the exact combination that causes this to break, it would help frame the problem more accurately, and would help speed a resolution.

Comment 19

[Dave Liebreich [:davel]]

I started an i386-only build of thunderbird on my laptop using the latest code on the 1.8.0 branch.

This build does not exhibit the bug.

I'm using gcc 4.0.1 with the 10.4u SDK

Here's my .mozconfig 

. $topsrcdir/mail/config/mozconfig

mk_add_options MOZ_CO_PROJECT=mail
mk_add_options MOZ_OBJDIR=@topsrcdir@/../objdir/mail

Where is the mozconfig for the universal build - I don't see it in /mofo/release/tinderbox-configs ...

Comment 20

[Dave Liebreich [:davel]]

I coped the mozconfig from the release tinderbox, removed the inclusion of build/macosx/universal/mozconfig, and built an i386-only binary.  the bug is still present.

Next step - remove ac_add_options --enable-optimize="-O2 -g"

Comment 21

[Dave Liebreich [:davel]]

bug is not present if build is not optimized.  changing summary line to reflect this information

Here's the .mozconfig file:

# Make flags
mk_add_options MOZ_CO_PROJECT=mail
mk_add_options MOZ_MAKE_FLAGS="-j4"
mk_add_options MOZ_CO_MODULE="mozilla/tools/update-packaging"
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/../objdir/tb

# Configure flags
ac_add_options --enable-application=mail

ac_add_options --enable-update-packaging
ac_add_options --enable-update-channel="release"

#ac_add_options --enable-optimize="-O2 -g"
ac_add_options --disable-debug
ac_add_options --disable-tests

ac_add_options --enable-official-branding

ac_add_options --enable-static
ac_add_options --disable-shared

ac_add_app_options ppc --enable-prebinding

Comment 22

[Dave Liebreich [:davel]]

the bug is present in optimized non-static build.

Comment 23

[Dave Liebreich [:davel]]

turning off optimization (CXXFLAGS += -O0) in mozilla/mailnews/news/src "fixes the bug.

Comment 24

[Mark Mentovai]

davel, thank you for testing!  Investigating...

Comment 25

[Daniel Veditz [:dveditz]]

We'll take this one based on a simple MacIntel-only news-only makefile tweak as in comment 23. Reassigning to mscott to get a patch as discussed on today's 1.5.0.4 triage meeting.

Comment 26

[Scott MacGregor]

Attachment: hack to turn off optimizations in mailnews\news\src

I'm starting to get suspicious about using this hack to work around the problem in 1.5.0.4, but here's the patch. Hopefully we'll have more information about the real issue by the end of the day today. 

I don't have an Intel mac to test this against to verify if it works or not.

Comment 27

[Scott MacGregor]

The compiler is not optimizing the for loop correctly at:

http://lxr.mozilla.org/mozilla/source/mailnews/news/src/nsNewsFolder.cpp#735

adding printf("char: %d", name[i]); 

makes the for loop compile correctly and work. Very strange.

Comment 28

[Scott MacGregor]

Attachment: patch to outfox the compiler optimization

storing name[i] in a variable enabled us to work around the compiler optimization. We can probably fix this on the trunk by re-writing the for loop, but this solution has the least impact and risk for 1.8.0.

I prefer this solution instead of disabling -O2 for news.

Anyone around tonight who can review this?

Comment 29

[Josh Aas]

Comment on attachment 222125 [details] [diff] [review]
patch to outfox the compiler optimization

This patch fixes the problem for me. Maybe put a short comment in there, can't hurt.

Comment 30

[Daniel Veditz [:dveditz]]

Comment on attachment 222125 [details] [diff] [review]
patch to outfox the compiler optimization

sr=dveditz
Shouldn't hurt a decent optimizer, and if it unconfuses this one that's great.

Comment 31

[Scott MacGregor]

Comment on attachment 222125 [details] [diff] [review]
patch to outfox the compiler optimization

per discussion with dan and rob.

Comment 32

[Scott MacGregor]

Technically it won't be fixed for 1.8.0.5 until Rob adjust the tag on the file and respins. So I'll leave that keyword off until that happens.

Comment 33

[Ludovic Hirlimann [:Usul]]

This looks like a compiler bug. Has this been reported to Apple ?

Comment 34

[Scott MacGregor]

Rob's in the process of re-tagging to pick this up for 1.8.0.5 so I'm going to add the fixed keyword now.

Comment 35

[Tim Riley [:timr]]

Comment on attachment 222125 [details] [diff] [review]
patch to outfox the compiler optimization

This should be approved for 1.8.0.4 not 1.8.0.5.

Comment 36

[Tim Riley [:timr]]

I am sure mscott meant 1.8.0.4 so I am changing the keyword.

Comment 37

[Tracy Walker [:tracy]]

verified with Tbird Mac UB 1.5.0.4rc3

Comment 38

[Shaohua Wen]

Dear all, 
   It seems that this bug fix caused regression of Bug 126453, Now both tb 2.0 and 1.5.0.4 have this regression.
   As I tested,If I subscribe a newsgroup with only first level(don't need abbreviatd name),It displayed correctly,but if the name is abbreviated,it becomes unreadable.
   could you please check that?

Comment 39

[Shaohua Wen]

See attachment https://bugzilla.mozilla.org/attachment.cgi?id=224430
actually,the first character is displayed correctly,but the others (after the first  dot "." )are become unreadable characters.

Comment 40

[Scott MacGregor]

Hi Holy, is the newsgroup name regression new in 1.5.0.4? i.e. did 1.5.0.2 have the problem? It seems unlikely that this particular change could have caused it, this change should behave exactly like the code before the patch.

Comment 41

[Shaohua Wen]

Attachment: tb1502 screen shot

Yes it's new in 1.5.0.4,Please find attached screenshot.
in the menus under news.newsfan.net,is the names of abbreviated names.

Comment 42

[Shaohua Wen]

(In reply to comment #40)
> Hi Holy, is the newsgroup name regression new in 1.5.0.4? i.e. did 1.5.0.2 have
> the problem? It seems unlikely that this particular change could have caused
> it, this change should behave exactly like the code before the patch.
> 
Is there any other bugs fixed in 1.5.0.4 that might change the behaviour of abbreviated name of newsgroup?

Comment 43

[Scott MacGregor]

This certainly seems like the most likely culprit. I'll take a look.

Comment 44

[Shaohua Wen]

(In reply to comment #43)
> This certainly seems like the most likely culprit. I'll take a look.
> 

Can you rollback this patch and make a build and provide a download address so that 
I can test it? 

Comment 45

[Scott MacGregor]

Holy,

These builds do not have this change:

http://archive.mozilla.org/pub/thunderbird/nightly/2006-05-14-11-mozilla1.8.0-l10n/
Please try this one to, this shouldn't have it but I wasn't sure:
http://archive.mozilla.org/pub/thunderbird/nightly/2006-05-15-13-mozilla1.8.0-l10n/

and these l10n builds do have this change:
http://archive.mozilla.org/pub/thunderbird/nightly/2006-05-16-19-mozilla1.8.0-l10n/

Can you test those three builds? Thanks!

Comment 46

[Shaohua Wen]

Attachment: 05-16-19 tb build

Hi Scott,
  I tried all these builds and found that they have the same build ID (20060504)  in the about Box.
  I also tried the build in http://archive.mozilla.org/pub/thunderbird/nightly/2006-05-17-10-mozilla1.8.0-l10n/ and get the same build ID.
  And all these builds are correct with the newsgroup abbreviated names.
  What could be wrong?
Best Regards,
Holy

Comment 47

[Scott MacGregor]

I forgot the l10n builds were probably all pulling final tagged bits for the 1.5.0.4 release at that time. You might have to test the en-US nightly bits:

http://archive.mozilla.org/pub/thunderbird/nightly/2006-05-14-04-mozilla1.8.0/

and 

http://archive.mozilla.org/pub/thunderbird/nightly/2006-05-16-21-mozilla1.8.0/

Comment 48

[Shaohua Wen]

Attachment: tb 05-14 build (works with the abbreviated name)

Comment 49

[Shaohua Wen]

Attachment: tb 05-16 build (not work with the abbreviated name)

Hi Scott,
  Please find attached two screen shots,20060514 is working with the abbreviated names and 20060516 is not working.
BR,Holy

Comment 50

[Scott MacGregor]

Attachment: fix the l10n regression [fixed on the 1.8.0.5 branch]

I think this should fix the problem Holy is seeing. Pretty embarrassing :(.

name is a unicode string not a char string.

Comment 51

[Scott MacGregor]

Comment on attachment 229535 [details] [diff] [review]
fix the l10n regression [fixed on the 1.8.0.5 branch]

I've checked the regression fix into the 1.8.1 branch and the trunk. Holy, can you test these bits when they come out tomorrow (7/18 Pacific time):

ftp://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-mozilla1.8

I will nominate for 1.8.0.6 if these builds work for you.

Comment 52

[Shaohua Wen]

(In reply to comment #51)
> I will nominate for 1.8.0.6 if these builds work for you. 
> 

Thanks Scott,I just tried and it works (build id 20060717). 

Comment 53

[Scott MacGregor]

Nominating for 1.8.0.6, since Holy just verified the fix for us. 

I'm also going to add a late nomination for 1.8.0.5 just in case we have to respin 1.8.0.5 so we can take this very low risk fix to address a regression introduced for supporting universal binaries. The regression effects all platforms, not just UB.

Comment 54

[Daniel Veditz [:dveditz]]

Comment on attachment 229535 [details] [diff] [review]
fix the l10n regression [fixed on the 1.8.0.5 branch]

approved for 1.8.0.5 since it's safe and we're respinning. a=dveditz for drivers

Comment 55

[Scott MacGregor]

Comment on attachment 229535 [details] [diff] [review]
fix the l10n regression [fixed on the 1.8.0.5 branch]

This actually already landed on the 1.8.1 branch the other day. I don't have access to adjust the approval1.8.1 flag.

Comment 56

[Scott MacGregor]

Comment on attachment 229535 [details] [diff] [review]
fix the l10n regression [fixed on the 1.8.0.5 branch]

Marcia, to verify this:

1) Make sure on UB mac builds, the newsgroup names are abbreviated properly (and not n.p.m.a)

2) Holy can probably help verify the l10n portion when it comes out on Windows with Chinese newsgroups. She's already verified the fix for the trunk and the 1.8.1 branch.

Comment 57

[Shaohua Wen]

(In reply to comment #56)
> 2) Holy can probably help verify the l10n portion when it comes out on Windows
> with Chinese newsgroups. She's already verified the fix for the trunk and the
> 1.8.1 branch.
> 
No problem,I can verify the l10n portion,and btw,I am a he....

Comment 58

[Scott MacGregor]

Sorry about that Holy. 

English bits for 1.5.0.5 that now have the fix can be found here:

ftp://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-mozilla1.8.0/thunderbird-1.5.0.5.en-US.win32.installer.exe

If you prefer an l10n build to test against, the fix can be found here:

ftp://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-mozilla1.8.0-l10n

make sure you grab one of the builds made on 7/19 or later and not the old 1.5.0.2 builds sitting in that directory. Thanks!

Comment 59

[Shaohua Wen]

(In reply to comment #58)
> If you prefer an l10n build to test against, the fix can be found here:
> 
> ftp://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-mozilla1.8.0-l10n
> 
l10n builds with buildid 20060719 tested and works,Thanks!

Comment 60

[Jay Patel [:jay]]

v.fixed on 1.8.0 branch based on last comment.  Looks like one of the original reporters is happy with the latest 1.5.0.5 rc4 builds. 

I was unable to verify this myself, since I can't seem to get the truncated newsgroup names to display like in the screenshots.

If anyone else was able to reproduce this, please retest and leave a comment here to let us know if things look ok now.  The latest "official" candidate builds for 1.5.0.5 are available here: ftp://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/1.5.0.5-candidates/rc4

Comment 61

[Shaohua Wen]

(In reply to comment #60)
> v.fixed on 1.8.0 branch based on last comment.  Looks like one of the original
> reporters is happy with the latest 1.5.0.5 rc4 builds. 
I retested rc4, and it's not working ,the buildid is wrong: 20060706

Comment 62

[Jay Patel [:jay]]

Shaohua:  Looks like that directory has some older build in there (they *should* all be the latest builds from 7/19).  So as long as the build you tested with was 20060719, then we're still good. :-)  So I think your comment #59 is valid and it is workign with rc4.

Comment 63

[Jay Patel [:jay]]

It looks like you are testing on Windows and the builds for that platform have not been updated in the rc4 directory on the ftp site for some reason.  We will look into that.

Comment 64

[Shaohua Wen]

(In reply to comment #63)
Yes I'm testing the windows builds

Comment 65

[Robert Helmer [:rhelmer]]

(In reply to comment #63)
> It looks like you are testing on Windows and the builds for that platform have
> not been updated in the rc4 directory on the ftp site for some reason.  We will
> look into that.
> 

This is right.. they will be updated later tonight, sorry for the confusion!

Comment 66

[Robert Helmer [:rhelmer]]

(In reply to comment #61)
> (In reply to comment #60)
> > v.fixed on 1.8.0 branch based on last comment.  Looks like one of the original
> > reporters is happy with the latest 1.5.0.5 rc4 builds. 
> I retested rc4, and it's not working ,the buildid is wrong: 20060706

Thank you for catching this problem! Would you mind retesting?

ftp://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/1.5.0.5-candidates/rc4

Comment 67

[Shaohua Wen]

(In reply to comment #66)
> Thank you for catching this problem! Would you mind retesting?
> 
> ftp://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/1.5.0.5-candidates/rc4
> 
You are welcome,
I just retested with the windows version and the buildid is 20060719,and it works,Thanks!

Comment 68

[Magnus Melin [:mkmelin]]

The patches were also checked in to trunk a looong time ago, marking FIXED.