5947 - Preferences are not read on Mozilla Mac

Status: VERIFIED DUPLICATE of bug 5132

(Core :: JavaScript Engine, defect, P1)

Description

[Peter Van der Beken]

Steps to reproduce:
Using a Mac build:

1) enter proxy preferences in the user50.js preference file
2) launch the browser
3) you can't access non-local pages, because the proxy information is not loaded
4) could be tested with other prefs (i found out with proxy settings)

The preferences are not being read on the Mac version of Mozilla. Aparently,
things go wrong in the function pref_InitInitialObjects() in
module:libpref:src:mac:macpref.cp at line 114. The statement "JSBool ok =
pref_ReadResource(3000);" always returns False in "ok", thus none of the other
ReadResources lines get called and the user preference file also doesn't get
read in.
I tried following execution from prefReadResource, and i think the resource gets
read in ok but the Javascript evaluation fails. That part is so complicated
though that i stopped looking when Javascript kicks in. So i'm not completely
sure this is a libPref or a JavaScript bug.

This was checked to be reproducible on MacOS 8.5.1 with the 5-5-1999 build, and
was seen in earlier builds too.

Haven't tested on other platforms.

Comment 1

[don]

John ...

Comment 2

[mcmullen]

This actually looks fairly serious. Moving back to M6

Comment 3

[pmock]

According to John, Mail is also broken because mail depends on reading the
pref50.js file.

Comment 4

[mcmullen]

Fixed. This fix will appear in today's final Macintosh build.

Comment 5

[pmock]

Using the May 5 1999-05-05-15 build, the preference file is now being read on
the Mac.  Mail folders and messages now appears again.
  This bug is verified on Mail.
Leaving this bug for Paulmac to verify on the browser issue.

Great work John! :)

Comment 6

[Paul MacQuiddy]

Verified - you can set prefs via the UI and they show up just swell the next
time you run.

Comment 7

[Peter Van der Beken]

I have found what the problem with prefReadResource() is: in two of the

javascript configuration files ("modules:libpref:src:initpref.js" and

"modules:libpref:src:init:all.js") there are blank lines at the end of the file,

after the last javascript statement. Removing these lines allows the

pref_ReadResource function to succeed. So the fix is simple: make it so that

those javascript files have only one newline after the last statement.



IMHO the extra lines are something that the javascript parser should be able to

deal with, so with the approval of John McMullen, i'm reopening this bug

assigned to JavaScript.

The bug is that on the Mac build, the JavaScript parser fails if there are blank

lines at the end of a .js file.

I haven't tested this on other platforms, but i tried changing the line endings

to Unix \n (instead of Mac \r) and the extra lines didn't choke the parser then.

It's got to do with detecting new lines but only mac-style new lines.

Comment 8

[mcmullen]

Removing the word "stopper" from the summary.

Comment 9

[Peter Van der Beken]

The bug is in jsscan.c in the function GetChar. When this function reads the
last character which on Mac is a CR in these preference files, it returns a CR
then a LF and then it starts returning characters which are not from the file
(where do they come from?). It should return a CR, a LF and then an EOF. If you
change the line endings in the JS files to UNIX the last character is a LF, it
returns a LF and then an EOF, which is correct.
This bug causes a variety of other Javascript compilation errors in perfectly
valid JS files. This can be seen when the files referenced from the XUL files
(eg navigator.js, referenced from navigator.xul) are read during initialisation.

Comment 10

[mcmullen]

Adding myself as cc:

Comment 11

[Mike McCabe]

First of all, thanks for the great exchange on this bug report.  I just stood
back and watched the facts come in... :)

I've been able to reproduce this xp.  In the standalone javascript engine (gmake
-f Makefile.ref in /mozilla/js/src) executing

eval("print('hello, world')\r\r");

... results in an error.

sil.js:4: illegal character:
rint('hello, world')

sil.js:4: ......................^

(Change 'print' to 'alert' if you're in the browser.)  Depending on the eval
string, I might also get other errors, like 'unterminated string'.  It looks
like the engine is indeed getting garbage characters.  The error goes away when
those \rs are changed to \ns, or with just one \r.

Setting the status to assigned.

Comment 12

[Mike McCabe]

Checked in a fix.

(aargh, forgot to credit the excellent sleuthing in the log.  Sorry!)

Turns out that this problem appeared when we removed the JSFILE (read js source
from a filehandle) #ifdef.  In the from-a-file case (only) we read the file into
a circular buffer.  js handles \r \n \r\n by collapsing to \n in the buffer, but
it has to special-case \r ending a buffer.  The code ran into trouble when \r\r
ended the buffer; it tried to advance the reading pointer past the end marker,
and then (because of an ==, not >, check) it merrily read on from there.

Fixed in two ways:

- This check now doesn't fire at all unless we're reading from a file.  This
means that all mozilla code prior to the removal of the JSFILE #ifdef is OK.
(jband?)  I'm sure this fixes the non-file case.

- Added a recursive GetChar() call to escape out of the \r buffer-ending check
and start over whenever we're in the accidental-overflow case.  I'm mostly sure
this fixes the file case.

Comment 13

[Paul MacQuiddy]

Pter, can you verify the fix? Thanks. Prefs still look good, but you were seeing
some initialization problems.

Comment 14

[Peter Van der Beken]

Prefs are read in, JS bug is gone. I think this bug is resolved. Last thing to
do: remove John's workaround in macpref.cp (line 109 to 111 and line 135).

Comment 15

[mcmullen]

Yes, and I have some changes in my tree. However, the remaining bug is really
subsumed under bug #5132. So I'm now marking this as a duplicate, in order to
make use of Bugzilla's nifty cross-linking feature.

Comment 16

[mcmullen]

Reopened, then marked as a duplicate.


*** This bug has been marked as a duplicate of 5132 ***

Comment 17

[leger]

Yep...a dup...Verified

Comment 18

[leger]

Changing component to "Javascript Engine".  "Javascript" component is being
retired.