Closed Bug 350055 Opened 14 years ago Closed 14 years ago

Win32 Sunbird should package urlformatter / Check packages-static for missing files

Categories

(Calendar :: Sunbird Only, defect)

x86
Windows 2000
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: ssitter, Assigned: ssitter)

References

Details

(Whiteboard: [no l10n impact])

Attachments

(2 files, 1 obsolete file)

Win32 Sunbird should package nsURLFormatter.js and urlformatter.xpt

With Bug 347944 urlformatter was added to toolkit as part of MOZ_XUL_APP.
[http://lxr.mozilla.org/mozilla/source/toolkit/components/Makefile.in]

Our linux builds already ship
  bin\components\nsURLFormatter.js
  bin\components\urlformatter.xpt

For win32 build they should be added to m/c/installer/windows/packages-static

Original changed Firefox only, Thunderbird already fixed it
[http://lxr.mozilla.org/seamonkey/source/mail/installer/windows/packages-static]
Thunderbird fix was done in Bug 349574. This might help with Bug 349729 too.
Blocks: 349729
Flags: blocking0.3+
I also compared the files produced by build in dist/bin with the files contained in resulting zip package. The following list shows the files that were not packaged:

bin/components/nsURLFormatter.js         <-- this patch
bin/components/urlformatter.xpt          <-- this patch

bin/components/jsconsole-clhandler.js    <-- Add ??? (handles -jsconsole flag)

bin/components/calWcapCalendarModule.js  <-- patch in bug 340949
bin/components/wcap.xpt                  <-- patch in bug 340949
bin/js/calWcapCachedCalendar.js          <-- patch in bug 340949
bin/js/calWcapCalendar.js                <-- patch in bug 340949
bin/js/calWcapCalendarItems.js           <-- patch in bug 340949
bin/js/calWcapErrors.js                  <-- patch in bug 340949
bin/js/calWcapRequest.js                 <-- patch in bug 340949
bin/js/calWcapSession.js                 <-- patch in bug 340949
bin/js/calWcapUtils.js                   <-- patch in bug 340949

bin/res/bloatcycle.html                  <-- ???
bin/res/svg.css                          <-- ???

bin/components/spellchecker.xpt          <-- ???
bin/dictionaries                         <-- ???
bin/dictionaries/en-US.aff               <-- ???
bin/dictionaries/en-US.dic               <-- ???

bin/README.txt                           <-- ???
bin/dependentlibs.list                   <-- ???

bin/uninstall                            <-- Add ???
bin/uninstall/uninstall.exe              <-- Add ???

We should check if this files must be added to packages-static too.
Assignee: nobody → ssitter
Status: NEW → ASSIGNED
Attachment #235315 - Flags: first-review?(mattwillis)
Comment on attachment 235315 [details] [diff] [review]
add both files to m//installer/windows/packages-static

(In reply to comment #2)
> bin/components/nsURLFormatter.js         <-- this patch
> bin/components/urlformatter.xpt          <-- this patch
This part looks good.

> bin/components/jsconsole-clhandler.js    <-- Add ??? (handles -jsconsole flag)
I'd check with jminta, but I don't think we really want to ship this.

> bin/components/calWcapCalendarModule.js  <-- patch in bug 340949
I checked in the changes for the WCAP files from this bug

> bin/dependentlibs.list                   <-- ???
> bin/res/bloatcycle.html                  <-- ???
These should be added to NO_PKG_FILES in mozilla/calendar/installer/Makefile.in

> bin/res/svg.css                          <-- ???
Add this beneath [Layout Engine Resources]

> bin/components/spellchecker.xpt          <-- ???
Add this to the components section

> bin/dictionaries                         <-- ???
> bin/dictionaries/en-US.aff               <-- ???
> bin/dictionaries/en-US.dic               <-- ???
Let's add bin\dictionaries\* to [@AB_CD@] as Firefox does

> bin/README.txt                           <-- ???
This is useless. I want to stop packaging it as it refers to Mozilla Suite. If someone were to actually read it, they'd probably become MORE confused. Let's remove this line from packages-static (and the similar one in the talkback section).

> bin/uninstall                            <-- Add ???
> bin/uninstall/uninstall.exe              <-- Add ???
The installer appears to create these itself? I don't think they need to be added, as it clearly has an uninstaller.

Since we're not packaging inspector (sigh), please also comment out the entries in beneath [Additional Developer Tools]. All they do is cause noise in tinderbox builds.

r=lilmatt with all that
Attachment #235315 - Flags: first-review?(mattwillis) → first-review+
(In reply to comment #3)
I'll attach an additional patch for the remaining issues later this day. 
The first patch can be checked in as is.
Summary: Win32 Sunbird should package nsURLFormatter.js and urlformatter.xpt → Win32 Sunbird should package urlformatter / Check packages-static for missing files
(In reply to comment #4)
> The first patch can be checked in as is.
Checked in.

(In reply to comment #3)
> > bin/components/jsconsole-clhandler.js    <-- Add ??? (handles -jsconsole flag)
> I'd check with jminta, but I don't think we really want to ship this.
> 
This looks pretty useful to me.  I think we should ship it.  Why wouldn't we?
Attached patch part 2: add missing files (obsolete) — Splinter Review
This patch adds the missing files to packages-static and comment out adt section.

Open points:
>> bin/dependentlibs.list, bin/res/bloatcycle.html
> These should be added to NO_PKG_FILES in> m/c/installer/Makefile.in

As long as we don't add them they are not shipped.

>> bin/README.txt
> This is useless. I want to stop packaging it as it refers to Mozilla Suite. 

Or package a Sunbird specific readme instead - like Firefox and Thunderbird does.

>> bin/uninstall/uninstall.exe
> The installer appears to create these itself? I don't think they
> need to be added, as it clearly has an uninstaller.

That seems right. From my few tests I can say that uninstall works. I'm just curious why uninstall.exe (and uninstall.ini) are listed in browser and mail packages-static file.
Attachment #235434 - Flags: first-review?(mattwillis)
Comment on attachment 235434 [details] [diff] [review]
part 2: add missing files

r=lilmatt
Attachment #235434 - Flags: first-review?(mattwillis) → first-review+
Attachment #235434 - Flags: second-review?(jminta)
Whiteboard: [patch in hand][waiting on jminta review]
Whiteboard: [patch in hand][waiting on jminta review] → [patch in hand][waiting on jminta review][no l10n impact]
Comment on attachment 235434 [details] [diff] [review]
part 2: add missing files

One thing I noticed later...

>Index: mozilla/calendar/installer/windows/packages-static
>===================================================================
> ; [Additional Developer Tools]
>-[adt]
>+;[adt]
Leave this line uncommented. That way, any part of the NSIS crap that happens to rely on there being an "adt" section won't barf, because it exists, even though it's empty.
Update due to recent changes in packages-static file and address previous comments. Carry over r1=lilmatt.
Attachment #235434 - Attachment is obsolete: true
Attachment #237021 - Flags: second-review?(jminta)
Attachment #237021 - Flags: first-review+
Attachment #235434 - Flags: second-review?(jminta)
Comment on attachment 237021 [details] [diff] [review]
part 2: add missing files, rev1

r2=jminta
Attachment #237021 - Flags: second-review?(jminta) → second-review+
Patch checked in on MOZILLA_1_8_BRANCH and trunk.

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [patch in hand][waiting on jminta review][no l10n impact] → [no l10n impact]
Verified. Files are contained in zip package and installer for Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060907 Calendar/0.3a2+
Status: RESOLVED → VERIFIED
No longer depends on: 350123
You need to log in before you can comment on or make changes to this bug.