219622 - SpellChecker component has no disk size

Status: VERIFIED FIXED

(SeaMonkey :: Installer, defect)

Description

[Serge Gautherie (:sgautherie)]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5) Gecko/20030916
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5) Gecko/20030916

SpellChecker component seems to have a disk size of "0 Kb";
with is wrong, I guess !


Reproducible: Always

Steps to Reproduce:
1. Custom Install Mozilla
2. In the component dialog, (un)select 'SpellChecker'.

Actual Results:  
Disk size remains unchanged.


Expected Results:  
Disk size should be updated as needed with the disk size of the spellchecker.


At it is, I believe that Installer is either under/over-estimating the needed
disk space :-(

Comment 1

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5) Gecko/20030916] (v1.5rc1) (W98SE)

Description is based on v1.5rc1/W98SE.

Comment 2

[Andreas Kunz]

In config.ini in the install package, the Spellchecker component has no "Install
Size Archive=..." extry, needed for display of download size. The other
components all have one.

This config.ini is created from config.it, I guess from here:
http://lxr.mozilla.org/seamonkey/source/xpinstall/packager/windows/config.it#635
This file _does_ have an entry for the archive size.
Replacement happens starting at
http://lxr.mozilla.org/seamonkey/source/xpinstall/packager/build/scripts/makecfgini.pl#146

http://lxr.mozilla.org/seamonkey/source/xpinstall/packager/build/scripts/makecfgini.pl#202
seems to expect a $InstallSizeSystem$ line following the $InstallSize$ one. The
next line is read, but in this case it's already the archive size line, so
nothing is done.
So the while loop is re-entered, reading the next line. The $InstallSizeArchive$
line has already been read, so whatever line is next is being processed now.
So
   elsif($line =~ /\$InstallSizeArchive\$/i)
is never entered for the spellchecker component and no line like
   "Install Size Archive=$installSizeArchive\n";
is written to config.ini. Which is exactly what we have and what the problem is.
The spellchecker component was added with version 1.138 of config.it by mkaply,
no bug number noted.

The solution would be to just add a $InstallSizeSystem$ line to the spellchecker
component. Version 1.45 of this config.it introduced the same fix, just for the
uninstaller component. There is also no bug number for it.

Other config.it files containing the spellchecker component are only the os2 and
the unix one. None of them has $InstallSizeSystem$ lines, so fixing
xpinstall/packager/windows/config.it should be the complete solution.

Patch follows, but I'll not ask for review before I (or anybody else) checked if
my theories are correct.

Comment 3

[Andreas Kunz]

Attachment: patch

Comment 4

[Andreas Kunz]

Comment on attachment 149680 [details] [diff] [review]
patch

Requesting r= from ssu and sr= from dveditz.

Patch was tested, created config.ini now says:
"Install Size Archive=267"
(with VC7.1), installer properly changes displayed download/install size when
(de-)selecting the spellchecker component.

Sean, can you propose a different superreviewer? A several weeks old similar
patch still did not get sr= from dveditz...

Comment 5

[Sean Su]

Comment on attachment 149680 [details] [diff] [review]
patch

r=ssu

I'll ping dan for you.

Comment 6

[Sean Su]

Comment on attachment 149680 [details] [diff] [review]
patch

I think dveditz is on vacation.  asking dougt for sr=

Comment 7

[Serge Gautherie (:sgautherie)]

Comment on attachment 149680 [details] [diff] [review]
patch

(In reply to comment #5)
> (From update of attachment 149680 [details] [diff] [review])
> r=ssu

Sean, you forgot to r+ the patch ;->


'approval1.7=?':
Trivial Packager data fix, no risk.

Comment 8

[Sean Su]

Comment on attachment 149680 [details] [diff] [review]
patch

r+'ing the patch

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in xpinstall/packager/windows/config.it;
/cvsroot/mozilla/xpinstall/packager/windows/config.it,v  <--  config.it
new revision: 1.141; previous revision: 1.140
done

Comment 10

[Andreas Kunz]

Leaving open until checked in to 1.7 branch. This is _really_ no-risk because
it's a patch for an input file of the installer packager only executed by
mozilla.org.

It fixes the displayed spell-checker size and fixes a possible problem when
Mozilla is installed to a partition with just not enough free space: up to now
the installer thinks there's enough space because it underestimates the required
space by some 100 kB.

Comment 11

[Asa Dotzler [:asa]]

Comment on attachment 149680 [details] [diff] [review]
patch

a=asa (on behalf of drivers) for checkin to Mozilla 1.7. We're wrapping things
up and so this will need to land today if it's going to make the release.

Comment 12

[Serge Gautherie (:sgautherie)]

(In reply to comment #11)
> (From update of attachment 149680 [details] [diff] [review])
> this will need to land today if it's going to make the release.

Christian (or anyone): Can you check it in on v1.7 branch ? Thanks.

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

fixed on 1.7 branch
Checking in xpinstall/packager/windows/config.it;
/cvsroot/mozilla/xpinstall/packager/windows/config.it,v  <--  config.it
new revision: 1.140.26.1; previous revision: 1.140
done

Comment 14

[Andreas Kunz]

Resolving as FIXED.

Comment 15

[Tracy Walker [:tracy]]

verified