141035 - cfg activation doesn't fail to launch browser if cfg is NOT present

Status: VERIFIED FIXED

(Core :: Preferences: Backend, defect)

Description

[Rodney Velasco]

In the Mozilla 0.9.4 (Netscape 6.2.1/6.2.2) timeframe we failed somewhat
"ungracefully" when we applied the CFG activation lines...

'pref("general.config.filename", "mozilla.cfg");'
'pref("general.config.vendor", "mozilla");'

to the all.js or all-ns.js and did not have a CFG present in the local install
directory.  This was a check made by chip clark on bug 5132 during the initial
implementation of CFG activation with the mozilla browser. 

Steps to reproduce:

1. Download latest trunk/branch build (i.e. 2002-04-29).

2. Edit the MOZILLA_INSTALL_DIR/defaults/pref/all.js (all-ns.js for netscape)
   and add the following lines to the file

   pref("general.config.filename", "mozilla.cfg");
   pref("general.config.vendor", "mozilla");

3. Start browser (we are testing for the failure case so no need to have a CFG
present in the MOZILLA_INSTALL_DIR)

Expected result:
Browser should FAIL to launch since we activated the CFG in all*.js file.  This
is to prevent end users from deleting a CFG and using an uncustomized browser.

Actual result:
Browser starts normally as if no CFG activation lines are present.

Last known build that worked was 200112220 builds. This could be a regression
caused by bug 89137

Comment 1

[tao]

nominate for rtm

Comment 2

[Brian Nesse (gone)]

There has been a lot of offline discussion regarding this bug. To summarize...

The application used to quit when this condition occurred because, as a part of
the preferences service initialization, this caused the preferences service to
throw an error in its Init() method. The failure of the preferences service to
startup propagated back up the calling chain, causing the application to quit.

Moving the autoconfig code out to a seperate module has pushed this check, and
the subsequent failure, off onto a parallel task... one which does not affect
the startup of the browser.

Subsequent investigation has shown that through the nsAppShellService there
appears to be a method which can be used to force the application to quit. The
problem with this is that if the quit message is processed before the
application enters the Run() loop it is effectively lost. I am still working to
find a workaround for this limitation.

Comment 3

[Brian Nesse (gone)]

Attachment: Preliminary patch

Here's a rough patch that causes mozilla to bring up an alert and quit. I'm
sure there are things that could be done better.

Comment 4

[Brian Nesse (gone)]

Attachment: Proposed patch

Here's a patch that is much better. The integration is a bit cleaner, and it
pulls the error text from localizable resources.

Comment 5

[Brian Nesse (gone)]

Adding Conrad and Samir for reviews. Conrad, could you review the AppShell 
changes? Samir, can you review the UI changes? Alec, sr?

This is an RTM blocker... time is of the essence. Thanks.

Comment 6

[tao]

Comment on attachment 86705 [details] [diff] [review]
Proposed patch


>Index: mozilla/extensions/pref/autoconfig/resources/content/contents.rdf
>===================================================================
>RCS file: resources/content/contents.rdf
>diff -N resources/content/contents.rdf
>--- /dev/null	1 Jan 1970 00:00:00 -0000
>+++ mozilla/extensions/pref/autoconfig/resources/content/contents.rdf	6 Jun 2002 19:43:39 -0000
>@@ -0,0 +1,18 @@
>+<?xml version="1.0"?>
>+<RDF:RDF xmlns:RDF="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
>+         xmlns:chrome="http://www.mozilla.org/rdf/chrome#">
>+
>+  <!-- list all the packages being supplied -->
>+  <RDF:Seq about="urn:mozilla:package:root">
>+    <RDF:li resource="urn:mozilla:package:autoconfig"/>
>+  </RDF:Seq>
>+
>+  <!-- package information -->
>+  <RDF:Description about="urn:mozilla:package:autoconfig"
>+        chrome:displayName="AutoConfig"
>+        chrome:author="mozilla.org"
>+        chrome:name="autoconfig"
>+ 	chrome:localeVersion="1.0.0">
The trunk is "1.0.0". now. But, the branch is "1.0.0final" now. You might want
to 
change the version # before landing it into branch.
>+  </RDF:Description>
>+
>+</RDF:RDF>
>
>Index: mozilla/extensions/pref/autoconfig/resources/locale/en-US/contents.rdf
>===================================================================
>RCS file: resources/locale/en-US/contents.rdf
>diff -N resources/locale/en-US/contents.rdf
>--- /dev/null	1 Jan 1970 00:00:00 -0000
>+++ mozilla/extensions/pref/autoconfig/resources/locale/en-US/contents.rdf	6 Jun 2002 19:43:39 -0000
>@@ -0,0 +1,23 @@
>+<?xml version="1.0"?>
>+<RDF:RDF xmlns:RDF="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
>+         xmlns:chrome="http://www.mozilla.org/rdf/chrome#">
>+
>+  <!-- list all the locales being supplied -->
>+  <RDF:Seq about="urn:mozilla:locale:root">
>+    <RDF:li resource="urn:mozilla:locale:en-US"/>
>+  </RDF:Seq>
>+
>+  <!-- locale information -->
>+  <RDF:Description about="urn:mozilla:locale:en-US">
>+    <chrome:packages>
>+      <RDF:Seq about="urn:mozilla:locale:en-US:packages">
>+        <RDF:li resource="urn:mozilla:locale:en-US:autoconfig"/>
>+      </RDF:Seq>
>+    </chrome:packages>
>+  </RDF:Description>
>+
>+  <!-- Version Information.  State that we work only with major version of this
>+       package. -->
>+  <RDF:Description about="urn:mozilla:locale:en-US:autoconfig"
>+	chrome:localeVersion="1.0.0"/>
Comments above applies here.

>+</RDF:RDF>
>

Comment 7

[Conrad Carlen (not reading bugmail)]

r=ccarlen on the appshell changes.

Comment 8

[Samir Gehani]

Comment on attachment 86705 [details] [diff] [review]
Proposed patch

r=sgehani on build/installer changes required for chrome registration.

Comment 9

[Alec Flett]

Comment on attachment 86705 [details] [diff] [review]
Proposed patch

sr=alecf

Comment 10

[Brian Nesse (gone)]

Adding keywords...

Comment 11

[Brian Nesse (gone)]

Checked in on the trunk.

Comment 12

[Mike Kaply [:mkaply]]

Can I ask a dumb question. Isn't the .cfg issue moot?

On 4.x you had to have a netscape.cfg or the browser wouldn't start. It got laid 
down by default by Netscape. A vendor could add locked prefs in it.

With Mozilla, all someone would have to do is remove the two lines from the text 
based js file and the browser would come up without the .cfg file.

This essentially makes the concept of the .cfg file (locked preferences) useless 
since anyone with an editor can turn it off...

Comment 13

[tao]

adt1.0.1+ per scott and syd's vote.

Comment 14

[Brian Nesse (gone)]

Michael, while it is true that a knowledgeable person can disable the reading of 
the .cfg file, I don't believe that the point is entirely moot. For one thing, 
the .cfg file can be used for many things besides just locking preferences. For 
instance, bug 121332 (blocked by this) is about accessing LDAP attributes. The 
testcase attached to that bug sets the users homepage to a value pulled from the 
the user's entry on an LDAP server.

In practice a vendor would probably also store info in the .cfg file that was 
necessary for the user to use the browser in their corporate environment. While 
the user could force the program to launch, they would effectively be left with a 
useless application.

Comment 15

[Brian Nesse (gone)]

Attachment: Mozilla packaging fix

Rodney has discovered that this doesn't work in a packaged build. I tracked it
down to a missing file (platform.js) which is not being copied over from the
viewer:defaults:autoconfig directory. This was apparently missed on a checkin
for bug 132140. The attached patch insures that all files in this directory are
grabbed in the packaging phase.

Comment 16

[Brian Nesse (gone)]

Rodney has also found that the error dialog is not appearing in the commercial 
build. This appears to be a regchrome issue. The bits are in the chrome packages, 
but are not being registered properly... still investigating.

Comment 17

[Daniel Veditz [:dveditz]]

Comment on attachment 87719 [details] [diff] [review]
Mozilla packaging fix

sr=dveditz

Comment 18

[Alec Flett]

Comment on attachment 87719 [details] [diff] [review]
Mozilla packaging fix

r=alecf

Comment 19

[Brian Nesse (gone)]

Attachment: Commercial patch

Patch to fix commercial chrome registration. Thanks to Dan for explaining to me
that chrome registration is unique, and does not take from Mozilla.

Comment 20

[Patrick C. Beard]

Comment on attachment 87728 [details] [diff] [review]
Commercial patch

sr=beard (rs)

Comment 21

[Rodney Velasco]

Dropping the platform.js in the defaults/autoconfig directory fixed the problem
on all platforms (mac OS 9.2.2, macOSX.1.5, win32, and Linux2.4-16) as far as
starting up the browser with the activation lines and CFG present in the install
directory (essential files for mac). This was tested on todays commercial trunk
builds. 

I'll try applying the patch for the error dialog/chrome registration issue I
reported to brian (see patch 87728) on windows and linux. I'll post my findings
once that build completes.

Comment 22

[Alec Flett]

Comment on attachment 87728 [details] [diff] [review]
Commercial patch

sr=alecf

Comment 23

[Brian Nesse (gone)]

Both patches checked in.

Comment 24

[Rodney Velasco]

New changes works on all supported platforms. Able to see failure dialog for all
using todays trunk builds, as well as startup with a valid CFG.

Verified on commercial trunk

Mac OS X (2002061708)
Linux 2.4.7-10 (2002061708)
Windows XP (2002061708)

Comment 25

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 26

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 27

[Brian Nesse (gone)]

New files added to the 1.0 branch... awaiting the go ahead from Leaf to land the
remainder and activate the code.

Comment 28

[Rodney Velasco]

over to luke for verification on the branch once it lands. 
Luke, please add verified1.01 to the keyword once you have completed testing this.

Comment 29

[tao]

Is this in branch already? if not, please check it in asap. thx!

Comment 30

[Brian Nesse (gone)]

As per comment #27, I am waiting for leaf to tell me that he has done whatever
CVS magic is required for new branch files before I can add them to the build. I
can not check in the remainder of the patch until this happens.

Comment 31

[tao]

hi, leaf: would you please do the CVS magic Brian needs? :-)

Comment 32

[Daniel (Leaf) Nunes]

actually, no magic needed doing. the files seemed to get added to the "right"
tag MOZILLA_1_0_BRANCH, i'll put them on MOZILLA_1_0_0_BRANCH now, too.

Comment 33

[Brian Nesse (gone)]

Remainder of the files checked in to the 1.0 branch.

Comment 34

[lrg]

I verified this bug on Windows 2000, Linux2.2 and MacOSX using todays (06/24)
branch Build, updating to verified1.0.1