347421 - [mac] xforms is part of the default build

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Marcia Knous [:marcia]]

Seen using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1b1) Gecko/20060804 BonEcho/2.0b1. Xforms is part of the default mac build. According to discussion this morning, it should not be.

Comment 1

[J. Paul Reed [:preed]]

Over to Rob; he said he'd have a chance to start investigating this afternoon.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Have you tested Windows and Linux to make sure this isn't true there as well?

Comment 3

[Marcia Knous [:marcia]]

It is not present in the Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060804 BonEcho/2.0b1 Windows build for sure. I will check Linux next.

(In reply to comment #2)
> Have you tested Windows and Linux to make sure this isn't true there as well?
> 

Comment 4

[Robert Helmer [:rhelmer]]

This started with the 2006-07-13-06-mozilla1.8 build. 
Maybe related to bug 344673?

Comment 5

[Robert Helmer [:rhelmer]]

(In reply to comment #4)
> This started with the 2006-07-13-06-mozilla1.8 build. 
> Maybe related to bug 344673?
> 

Actually 339568 is more likely.

Comment 6

[Mark Mentovai]

Yeah.  I accidentally put this comment on bug 339568.  Here it is:

The installed xforms extension has a ppc-only libxforms.dylib, where the xpi
has the proper fat version.  Note these warnings when doing the final unify:

/builds/tinderbox/Fx-Mozilla1.8/Darwin_8.7.0_Depend/mozilla/build/macosx/universal/unify:
warning: makeUniversalDirectory: only in ppc
../build/unifox/ppc/dist/firefox/BonEcho.app/Contents/MacOS/extensions/\{cf2812dc-6a7c-4402-b639-4d277dac4c36\}:
  chrome.manifest,
  components,
  chrome,
  install.js,
  install.rdf

The problem here is somewhat like the problem in bug 328854.  We make in
browser/app (only for ppc) after building Talkback in order to get Talkback
into the app bundle.  When XForms is also built, this additional make also has
the side effect of putting it into the bundle.  (In the case of 328854, it's
not another extension being built that's the problem, it's changes to the app
that occur while running tests.)

Perhaps we should build Talkback and remake in browser/app immediately after a
build completes, instead of waiting until other things like XForms and tests
are done.  (We'd still want to wait for tests to complete before uploading
symbols...)

Comment 7

[Mark Mentovai]

Actually, that's a sucky idea, because we'd run tests on an app with Talkback installed, and if it crashed, it would leave one more thing running on the system.  Leaving the CrashReporter dialog up is bad enough as it is.  It's mucked with test timings in the past.

(Actually, we should get rid of the CrashReporter dialog altogether on the Tinderboxen.  Run |defaults write com.apple.CrashReporter DialogType server| to suppress the dialogs.  Crash data will still be saved in ~/Library/Logs/CrashReporter.  Maybe you should do this on all the new boxes and add it to the instructions for setting up a reference system.)

Anyway.

It's probably best to not rely on |make -C browser/app| to get Talkback into the bundle after it's built.  We could rsync it directly by hand, to make sure that we get only Talkback and nothing else.  The only problem with this is that the Talkback component lives in different locations for different products, and can vary by branch.  The only thing that knows where Talkback is supposed to live is the Talkback Makefile.  Talkback lives at different locations in Fx/Tb since 1.5 than it does in Cm/Sm, for example.  I guess doing it this way will require another tunable in tinder-config.

If we changed things around to make rsync copy talkback directly into the universal staging area, we could also eliminate the need to run unify a second time, and solve bug 328854 in the process.

Comment 8

[J. Paul Reed [:preed]]

(In reply to comment #5)
> (In reply to comment #4)
> > This started with the 2006-07-13-06-mozilla1.8 build. 
> > Maybe related to bug 344673?
> > 
> 
> Actually 339568 is more likely.

Do we need to revert the patch for 339568 until it works correctly?

I hate to remove code like this, but lockdown for b2 is Tuesday night at 11:59, and I don't think we can ship b2 on the mac with this bug, so I think the quickest solution is reverting the patch for bug 339568.

Comment 9

[Mark Mentovai]

Pretty sure you can just turn BuildXForms off - you'd need to do that if you backed the patch out anyway.

Comment 10

[Robert Helmer [:rhelmer]]

(In reply to comment #9)
> Pretty sure you can just turn BuildXForms off - you'd need to do that if you
> backed the patch out anyway.
> 

I've turned this off on the branches until the problem is resolved.

Comment 11

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1b1) Gecko/20060808 BonEcho/2.0b1. I no longer see x-forms listed in the Add-Ons dropdown.

Comment 12

[Marcia Knous [:marcia]]

adding keyword, forgot to do it when I verified the bug.

Comment 13

[jhp (no longer active)]

(In reply to comment #7)
> The only thing that knows where Talkback is supposed to
> live is the Talkback Makefile.  Talkback lives at different locations in Fx/Tb
> since 1.5 than it does in Cm/Sm, for example.  I guess doing it this way will
> require another tunable in tinder-config.

Or how about just adding a rule in browser/app/Makefile.in to copy Talkback to the correct directory.  Then just do 'make -C browser/app talkback'.

Comment 14

[Marcia Knous [:marcia]]

seth just pointed out that xforms is back in today's build - Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.8pre) Gecko/20061019 Firefox/1.5.0.8pre.  Can we please remove and also try to ensure this doesn't happen in the future?

Comment 15

[(not reading, please use [e-mail] instead)]

thanks to basil for helping to spot this.

Comment 16

[Jay Patel [:jay]]

This is a blocker for Firefox 1.5.0.8.  We should NOT be shipping with XForms by default.

We also need to find out how this happened again... it looks like rhelmer fixed it in comment #10 by disabling it in the build scripts.  Did it somehow get reverted back?

In anycase, please get this fixed ASAP so we can test the new builds and tag the tree to 1508 RC1 (we hope to have RC1 builds by Monday 10/23).

Comment 17

[Jay Patel [:jay]]

re-assigning to TR since rhelmer is on vacation.

Comment 18

[T.R. Fullhart]

Attachment: Patch to turn off XForms in nightly macosx Fx

This patch turns off building XForms in the nightly Firefox MacOSX tinderbox.

Comment 19

[(not reading, please use [e-mail] instead)]

tr, any idea how this regressed?

Comment 20

[J. Paul Reed [:preed]]

Comment on attachment 242912 [details] [diff] [review]
Patch to turn off XForms in nightly macosx Fx

Looks good; r=preed

Comment 21

[T.R. Fullhart]

Attachment: Patch to turn off XForms in release macosx Fx

This explicitly turns off building XForms and should be applied to the MOZILLA_1_8_0_BRANCH_release of /mofo/release/tinderbox-configs.

Comment 22

[T.R. Fullhart]

(In reply to comment #19)
> tr, any idea how this regressed?
> 

I don't think it's a regression. As far as I can tell, it was already off for the releases, it was only on for the nightly but that might have been on purpose to do testing.

Comment 23

[T.R. Fullhart]

Checking in firefox/macosx/tinder-config.pl;
/mofo/release/tinderbox-configs/firefox/macosx/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.1.2.3.2.1.2.16; previous revision: 1.1.2.3.2.1.2.15
done

Comment 24

[T.R. Fullhart]

Checking in tinder-config.pl;
/mofo/release/tinderbox-configs/firefox/macosx/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.1.2.3.2.1.2.8.2.7; previous revision: 1.1.2.3.2.1.2.8.2.6
done

Comment 25

[T.R. Fullhart]

(In reply to comment #19)
> tr, any idea how this regressed?
> 

As far as I can tell, it was only turned off on certain branches. It never was turned off on the branch that the Fx-Moz1.8.0-universal tinderbox was using. It was turned off on the release branch, though.

Comment 26

[Hiro]

In trunk build, xform still exists. 
And bug346831 still reproduces. 
find Minefield.app -name xform*
Minefield.app/Contents/MacOS/extensions/{cf2812dc-6a7c-4402-b639-4d277dac4c36}/chrome/xforms.jar
Minefield.app/Contents/MacOS/extensions/{cf2812dc-6a7c-4402-b639-4d277dac4c36}/components/xforms.xpt

In tinderbox-builds/xserve06-trunk, this problem doesn't exist.  


Mac OS X 10.3.9
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20061025 Minefield/3.0a1

Comment 27

[Marcia Knous [:marcia]]

verified using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0. Adding keyword.

Comment 28

[Marcia Knous [:marcia]]

Cut and paste hiccup, I meant Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.8) Gecko/20061025 Firefox/1.5.0.8. 

(In reply to comment #27)
> verified using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1)
> Gecko/20061010 Firefox/2.0. Adding keyword.
> 

Comment 29

[Håkan Waara]

It seems that this problem now exists on trunk. Did it regress?

The "Default font" menu in the Content prefs is empty, and can not be used. Please see bug 360244

Comment 30

[:Gavin Sharp [email: [e-mail]]]

http://lxr.mozilla.org/mozilla/source/tools/tinderbox-configs/firefox/macosx/tinder-config.pl#17
seems to indicate that this has indeed regressed.

Comment 31

[T.R. Fullhart]

(In reply to comment #30)
> http://lxr.mozilla.org/mozilla/source/tools/tinderbox-configs/firefox/macosx/tinder-config.pl#17
> seems to indicate that this has indeed regressed.
> 

Gavin, as far as I can tell, XForms has been turned on for trunk OSX FF nightly tinderbox for a long time. AFAIK, XForms wasn't ever turned off for trunk nightlies on OSX. I assumed that it was because someone wanted to use it on the trunk or something.

Do you want it turned off for the trunk nightlies tinderbox? It's also turned on for MOZILLA_1_8_BRANCH nightlies. Do you want it turned off there, too?

XForms was already turned off for all release builds and MOZILLA_1_8_0_BRANCH nightlies.

Comment 32

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #31)
> Gavin, as far as I can tell, XForms has been turned on for trunk OSX FF nightly
> tinderbox for a long time. AFAIK, XForms wasn't ever turned off for trunk
> nightlies on OSX.

Ah, I see. I didn't know that having it enabled on the trunk was intentional. As long as it's disabled for release builds, I guess there isn't anything left to do in this bug. Feel free to resolve it!

Comment 33

[T.R. Fullhart]

Resolving per Gavin's comment. Just let us know if anyone wants it turned off for trunk nightlies.

Comment 34

[Adam Guthrie]

I don't think we need to be shipping with Xforms installed by default with /any/ builds, including Mac nightlies. 

If the problem of not being able to build Xforms on Mac without installing it still exists, another bug needs to be filed for that, but in the mean time we need to turn off "installing" Xforms on all builds.

Comment 35

[J. Paul Reed [:preed]]

(In reply to comment #34)

> If the problem of not being able to build Xforms on Mac without installing it
> still exists, another bug needs to be filed for that, but in the mean time we
> need to turn off "installing" Xforms on all builds.

That's bug 345992.

Feel free to reopen it.

In the meantime, is there a case to be made that XForms is hugely bad to nightly testers, other than the additional few K?

Comment 36

[Zach Lipton [:zach]]

Last time we went through this, iirc there were XForms-related bugs that caused confusion when testers were seeing bugs on mac with xforms that couldn't be reproduced on other platforms without xforms. There's not any kind of urgent need for it to be turned off I don't think, and perhaps fixing 345992 is the right way to make this happen, but the problem is more then the few extra KB.

Comment 37

[Uri Bernstein]

(In reply to comment #35)

> In the meantime, is there a case to be made that XForms is hugely bad to
> nightly testers, other than the additional few K?
> 

For one, it apparently causes bug 346831.

Comment 38

[(not reading, please use [e-mail] instead)]

xforms is back again, at least on the trunk.

I just got "Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a3pre) Gecko/20070325 Minefield/3.0a3pre" from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2007-03-25-04-trunk/firefox-3.0a3pre.en-US.mac.dmg

and it included xforms.

Note, it could be in earlier nightly trunk builds, I haven't checked yet.

Thanks for justin for point this out.

could this be a regression in some sort of build configuration?

Comment 39

[Uri Bernstein]

(In reply to comment #38)
> xforms is back again, at least on the trunk.
> 
According to comment #31 (and comment #32), this is intentional (which is why this bug was closed).

Comment 40

[(not reading, please use [e-mail] instead)]

oops, thanks Uri.  Nick Thomas pointed out the same thing.

sorry for the noise.

Comment 41

[Tracy Walker [:tracy]]

Just because the tinderbox config has it turned doesn't mean it should be on. As the trunk is now a release branch.  Turning Xforms off just for the actual released builds doesn't make sense from a QA perspective. This is especially so if xforms is hiding bugs that would only be able to be caught in the short pre-release regression testing period. 

reopening, it's in
Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.9a4pre)
Gecko/20070330 Minefield/3.0a4pre

Comment 42

[Marcia Knous [:marcia]]

I think this bug needs a new assignee since TR is no longer around.

Comment 43

[Marcia Knous [:marcia]]

I agree with Tracy here.  The trunk is complicated enough, having Xforms in the mix as a wild card during QA testing is not an optimal situation. I would prefer having it turned off, especially as we get further into the development cycle.

(In reply to comment #41)
> Just because the tinderbox config has it turned doesn't mean it should be on.
> As the trunk is now a release branch.  Turning Xforms off just for the actual
> released builds doesn't make sense from a QA perspective. This is especially so
> if xforms is hiding bugs that would only be able to be caught in the short
> pre-release regression testing period. 
> 
> reopening, it's in
> Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.9a4pre)
> Gecko/20070330 Minefield/3.0a4pre
> 

Comment 44

[J. Paul Reed [:preed]]

We can turn XForms off on the trunk for the mac, but we should let the XForms guys know we're doing this (and optionally, reopen bug 345992, so they can track fixing it themselves).

I agree that it's a packaging bug they need to fix, and if it's causing (or obscuring) Mac bugs trunk bugs, that's not optimal.

Comment 45

[jhp (no longer active)]

(In reply to comment #44)
> I agree that it's a packaging bug they need to fix...

Actually, as mentioned in comment #6 and comment #7 by mento, this is a Talkback packaging bug that the XForms build process exposes.

Comment 46

[Tracy Walker [:tracy]]

fixed in bug 377198

Comment 47

[Adam Guthrie]

I guess this is fixed now... ->FIXED