Closed Bug 364599 Opened 18 years ago Closed 17 years ago

Some new created profile files are write protected, for example bookmarks.html and localstore.rdf

Categories

(Mozilla Localizations :: de / German, defect)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: broedli, Assigned: moco)

References

()

Details

(Keywords: regression, verified1.8.0.10, verified1.8.1.2)

Attachments

(13 files, 6 obsolete files)

981 bytes, text/plain
Details
1.92 KB, text/plain
Details
2.76 KB, patch
rhelmer
: review+
Details | Diff | Splinter Review
5.58 KB, patch
benjamin
: review+
dveditz
: review-
Details | Diff | Splinter Review
2.15 KB, patch
dveditz
: review+
Details | Diff | Splinter Review
5.47 KB, patch
dveditz
: review+
Details | Diff | Splinter Review
1.74 KB, patch
dveditz
: review+
Details | Diff | Splinter Review
1.75 KB, text/plain
Details
1.80 KB, patch
moco
: review+
Details | Diff | Splinter Review
2.63 KB, text/plain
Details
1.75 KB, patch
moco
: review+
Details | Diff | Splinter Review
2.81 KB, patch
Details | Diff | Splinter Review
2.63 KB, patch
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1

Firefox 2.0.0.1 de creates files like bookmarks.html and localstore.rdf write protected in the profile folder, so Firefox is unable to save some prefrences, like window size.

Reproducible: Always

Steps to Reproduce:
1. Install Firefox 2.0.0.1 de (german)
2. Create a new profile or delete bookmarks.html and localstore.rdf in the existing profile
3. start Firefox
Actual Results:  
The new created files bookmarks.html and localstore.rdf are write protected

Expected Results:  
bookmarks.html and localstore.rdf should not be write protected

After a installation of Firefox 2.0.0.1 de the files in the folder *\Mozilla Firefox\defaults\profile are write protected, so I think this is the reason for this problem. If I remove the write protection, Firefox creates these files
without a write protection in the profile folders.

I could reproduce this behaviour with Firefox 2.0.0.1 de under Windows XP Home and Windows XP Professional with different installation parameters. With Firefox 2.0 de, Firefox 2.0 en-US, Firefox 2.0.0.1 en-US the files in *\Mozilla Firefox\defaults\profile and new created profile files are not write protected.
*** Bug 364752 has been marked as a duplicate of this bug. ***
Problem has been verified for those versions :

Release Installer of FX 2.0.0.1 de-DE Win32 and also Linux

Nighly Installer Build-Localisations of FX 2.0.0.1 de-DE

also the candidate rc3 of the Installer 2.0.0.1 de-DE

both on Win32

all these Installer will set a write protection on the default profile
files (\Mozilla Firefox\defaults\profile) after installation. 
After making a new profile the write protection attribute will 
transfered to the new profile.

Pike: ideas? Ideas about who would have ideas?
Flags: blocking1.8.1.2?
Version: unspecified → 2.0 Branch
I guess the right people are on vacation ;-)

since this bug is solveable (just remove the write lock) we have to wait for 2.0.0.2

http://camp-chaos.net/deutsch/misc/firefox_bug_in_201/ 
Confirmed by several posts in the German Community Forum (http://www.firefox-browser.de/forum/viewtopic.php?t=45176)
Status: UNCONFIRMED → NEW
Ever confirmed: true
I don't have a good idea here. I doubt it's installer right now, could someone verify this bug with a candidate zip?

I'd like to get tests on a matrix, 1.5.0.x, 2.0, 2.0.0.1, en-US, de, and another locale, on windows, mac, linux. Yeah, lots of tests, but I won't have resources to look into this this year in any detail.
Thinking about it, testing this on 1.5.0.x corresponding to 2.0 and 2.0.0.1 might give some clues, too.
are there any comlains within the italian version or any other local language version?
cc'ing a couple of people that *might* be able to help.  But we do need to find an owner for this bug and get a fix for 2.0.0.2.
Flags: blocking1.8.1.2? → blocking1.8.1.2+
(In reply to comment #7)
> I don't have a good idea here. I doubt it's installer right now, could someone
> verify this bug with a candidate zip?

Its only a Problem with the Installer Build, the Zip File from 2001 RC3 is fine (Mozilla/5.0 (Windows; U; Windows NT 5.2; de; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1)

Steps to Reproduce :

If you have already 2001 installed, deinstall via the Windows Control Panel:

1. Rename your Profile 
2. Install/Download your Firefox DE Build.
3. Start Firefox (it will start with the Default Profile).
4. Add some Bookmarks to the Bookmarks Menu and Toolbar (you will notice this error also, because the Add-Bookmark Dialog won`t go away after you press okay, when you add an Bookmark).
5. Verify you have several! Bookmarks and Bookmarks on Toolbar.
6. Restart Firefox 

-> Your Bookmarks are lost and your Toolbar has only 1 of your Bookmarks.

Also you run into http://kb.mozillazine.org/Multiple_bookmarks_files_created

The following files in my profile were set to read-only:
bookmarks-2006-12-28.html (\Profiles\8qbk38u6.default\bookmarkbackups)
Profiles\8qbk38u6.default\chrome <- all files
bookmarks.html.moztmp
localstore.rdf
mimeTypes.rdf
search.rdf

Notes:
-> When you create a second profile (via Firefox -p) the same Problem occur.
-> If you have already a Profile (like from Firefox 2), and don`t rename your Profile during the 2001 Installation, you don`t run into this Error. You can add then Bookmarks and do Toolbar Customization of your choice with no problems/errors.
-> The DE Installer for Firefox2 is okay
-> Firefox 1509DE Installer is also okay.

As far as i can see, its no Problem with the Install Build on en-US, ru and fr. I will test also several other locales.
Keywords: regression
also not reproduce able on Linux Fedora FC6, but on Windows XP x64 US
It appears that several of the de l10n files were checked in with the read-only attribute set. Someone with access to the l10n tree will need to remove the read-only attribute and then check them in again to fix this bug.
I can reproduce this under Linux (Ubuntu 6.10). If i extract the archive with --preserve-permissions (default for root) all files in the directory /firefox/defaults/profile are read only and firefox creates these files also read only in the profile folders.

/firefox/defaults/profile:
-r--r--r-- 1 root root 7137 2006-09-28 19:06 bookmarks.html
drwxr-xr-x 2 root root 4096 2006-12-09 00:53 chrome
-r--r--r-- 1 root root  153 2005-07-24 19:52 localstore.rdf
-r--r--r-- 1 root root  287 2005-07-24 19:52 mimeTypes.rdf
-r--r--r-- 1 root root 3287 2005-07-24 19:52 search.rdf

profile folder:
-r--r--r-- 1 *** ****    7137 2006-12-28 22:50 bookmarks.html
-r--r--r-- 1 *** ****    153 2006-12-28 22:50 localstore.rdf
...

With Firefox 2.0.0.1 en-US:

/firefox/defaults/profile:
-rw-r--r-- 1 root root 7138 2006-12-09 00:09 bookmarks.html
drwxr-xr-x 2 root root 4096 2006-12-09 00:09 chrome
-rw-r--r-- 1 root root  153 2006-12-09 00:09 localstore.rdf
-rw-r--r-- 1 root root  287 2006-12-09 00:09 mimeTypes.rdf
-rwxr-xr-x 1 root root  347 2006-12-09 00:09 prefs.js
-rw-r--r-- 1 root root 3287 2006-12-09 00:09 search.rdf

profile folder:
-rw-r--r-- 1 *** ****    7165 2006-12-28 22:57 bookmarks.html
-rw-r--r-- 1 *** ****    1026 2006-12-28 22:57 localstore.rdf
...
-> DE , because this seems to be a Bug in the DE Version (Read Only Attribute in Installer Files, see comment #12) and not a general Bug in the Installer.

Also other locales are fine.
Assignee: nobody → kairo
Component: Installer → de-AT / German-Austria
Product: Firefox → Mozilla Localizations
QA Contact: installer → mozilla
Version: 2.0 Branch → unspecified
Assignee: kairo → a.topal
(In reply to comment #12)
> It appears that several of the de l10n files were checked in with the read-only
> attribute set. Someone with access to the l10n tree will need to remove the
> read-only attribute and then check them in again to fix this bug.
> 

As you can see from lxr [1] there was no checkin to those files after the release of Firefox 2.0, which apparently works. Except bookmarks.html they haven't been touched since their initial checkin. There must be a problem somwhere in the packaging of the installer. 

This is a pretty serious issue and we should try to fix it as soon as possible, waiting for 2.0.0.2 is no option for us. Thousands of user are downloading Firefox everyday in German and we can't leave them with a half working browser.

[1] http://lxr.mozilla.org/l10n-mozilla1.8/source/de/browser/profile/
Whiteboard: Workaround for German User described here http://www.firefox-browser.de/wiki/Schreibschutzfehler_in_2.0.0.1
Does this problem exist in FF2?  If not does it exist if you do a 2->2.0.0.1 update?  If that update path works properly we should pull 2.0.0.1-de from the website and replace it with 2.0.0.0 until we can get a fixed version out.  Those users will get the 2.0.0.1 update within 24 hours so it's not a huge deal.

Whiteboard: Workaround for German User described here http://www.firefox-browser.de/wiki/Schreibschutzfehler_in_2.0.0.1
Whiteboard: Workaround for German User described here http://www.firefox-browser.de/wiki/Schreibschutzfehler_in_2.0.0.1
This isn't going to show up in ZIP or MAR builds because those builds don't preserve readonly bits (at least as far as I know).

Is this still a problem with the repacks being produced from the tinderbox? The readonly bits could theoretically have been introduced in several places:

1) doing l10n repacks from readonly sources
2) doing signing repacks

It would be good to write a release verification test for this (I can't imagine that any of our release files should be marked readonly).
OS: Windows XP → All
Hardware: PC → All
(In reply to comment #12)
> It appears that several of the de l10n files were checked in with the read-only
> attribute set. Someone with access to the l10n tree will need to remove the
> read-only attribute and then check them in again to fix this bug.

Here is what I've tested so far:

2.0 win32/linux - no problems
2.0.0.1 win32/linux - same files that are read-only in l10n/de are read-only (doesn't seem to be a problem on mac, but I haven't tested the archive yet)
1.5.0.9 linux - same problem (win32 installer uses .xpi files so permissions are not preserved, works as expected)
trunk win32 - same problem

I don't see any tinderbox changes that went in during this time, and I don't think that all of the machines responsible for the above builds have been changed in a way that would affect this between the 2.0 and 2.0.0.1 releases.

Since in CVS the mode is set on the RCS files and is not versioned, it makes sense that it would be a consistent problem across branches and platforms like this, if the mode in the repo is the problem.

There are some files set read-only in CVS that are not in the build, and vice versa, which seems inconsistent with this theory though:

Here are the read-only files in the build:

./localized/defaults/profile/chrome/userChrome-example.css
./localized/defaults/profile/chrome/userContent-example.css
./localized/defaults/profile/bookmarks.html
./localized/defaults/profile/localstore.rdf
./localized/defaults/profile/mimeTypes.rdf
./localized/defaults/profile/search.rdf
./localized/searchplugins/amazondotcom-de.xml
./localized/searchplugins/eBay-de.xml
./localized/searchplugins/wikipedia-de.xml
./localized/searchplugins/yahoo-de.xml
./localized/README.txt

The files in CVS (which are only in l10n/de) include several files in browser/chrome, plus:

./browser/installer/installer.inc
./browser/profile/bookmarks.html
./browser/searchplugins/eBay-de.xml
./browser/searchplugins/wikipedia-de.xml
./browser/searchplugins/yahoo-de.xml
./browser/searchplugins/amazondotcom-de.xml
./browser/searchplugins/list.txt
Just unpacked the (Mac) DMG for 2001, no read-only files inside.
If .xpi files are not preserving permissions that's a bug, one that we've had to fix in the past.
(In reply to comment #20)
> It would be good to write a release verification test for this (I can't imagine
> that any of our release files should be marked readonly).

I've put a patch for this in bug 365725 after verifying that it finds this problem.

Is this fixable for folks that have the perms problem either through a special case in the 2.0.0.2 update or through a separate bash/bat script?
I posted a relnote/readme on mozillaZine that should help others avoid this issue:  http://forums.mozillazine.org/viewtopic.php?p=2680938
(In reply to comment #26)
> I posted a relnote/readme on mozillaZine that should help others avoid this
> issue:  http://forums.mozillazine.org/viewtopic.php?p=2680938
> 

The translated text for the German Users can be found here http://www.firefox-browser.de/forum/viewtopic.php?p=336950#336950
Whiteboard: Workaround for German User described here http://www.firefox-browser.de/wiki/Schreibschutzfehler_in_2.0.0.1 → http://www.firefox-browser.de/forum/viewtopic.php?p=336950#336950
I was directed here by another bug which seems similar to mine. In the English version of Firefox 2.0.0.1, when I try to bookmark a page, the bookmark menu comes up, I click OK and the button seems to stick for a couple seconds. The bookmark appears in the list but upon restart of Firefox is gone. Reproducible always on Windows XP SP2.
(In reply to comment #28)
> I was directed here by another bug which seems similar to mine. In the English
> version of Firefox 2.0.0.1, when I try to bookmark a page, the bookmark menu
> comes up, I click OK and the button seems to stick for a couple seconds. The
> bookmark appears in the list but upon restart of Firefox is gone. Reproducible
> always on Windows XP SP2.
> 

This Bug here is (as far as i know) only in the DE Firefox Version.
Maybe you see http://kb.mozillazine.org/Bookmarks_not_saved
The problem seems to be that there are CVS watches set on several files in l10n/de, in the l10n repository. This causes the watched files and directories to be checked out read-only (for use with the "cvs edit" command).

I created some l10n builds using a local mirror to test this, and the DE build does not have this problem anymore:
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox/cerberus-firefox2.0.0.1-l10n

I set the l10n tinderbox to do a clobber build just to make sure, I'll send out a link to it in the morning.
works for me on the clobber build, no Readonly files under the profile or under \defaults\profile
Severity: normal → critical
Thanks Tomcat!

Here is a link to the builds, in case anyone else would like to test:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2007-01-02-22-firefox2.0.0.1-l10n/

These builds pass the usual l10n verification, with the read-only-test patch from bug 365725.

These builds were produced using a CVS mirror, with all of the "fileattr" files removed (which removes the watches). This can be done the cvs client by making sure there are no editors and then running "cvs watch off" and "cvs watch remove".

Pike, can you make sure this happens?
What worries me most here is how this could happen for only German builds. They should be produced exactly the same way that other localizations are, from CVS, only difference being the content of text files. My fear is that they are produced on different machines, or that whatever caused this hickup might just as well have caused a hostile binary change.
rhelmer, are there still active watchers in the l10n repo? It'd be interesting to find out who's doing that in the first place, and why.

If watches or editors could break our build, I wonder if we can disable them completely.
(In reply to comment #34)
> If watches or editors could break our build, I wonder if we can disable them
> completely.

bsmedberg pointed out elsewhere that we probably need a fix at the build-system level to copy these files with the desired permissions.

I think this would be a better fix overall than crippling CVS and/or depending on release verification scripts (which won't catch problems until after RCs have been generated).
Thanks to Phil, I learned about cvs watchers and editors.

Jose Sun, could you please remove yourself from being editor and watcher all over the l10n repository? It turns out that that is a bad idea.
I have never found this incident. I've "unedit" all over in the l10n repo.
Please check again, Axel.
Agreed - would be good for the build scripts to enforce permission issues at build time.   

(In reply to comment #35)
> (In reply to comment #34)
> > If watches or editors could break our build, I wonder if we can disable them
> > completely.
> 
> bsmedberg pointed out elsewhere that we probably need a fix at the build-system
> level to copy these files with the desired permissions.
> 
> I think this would be a better fix overall than crippling CVS and/or depending
> on release verification scripts (which won't catch problems until after RCs
> have been generated).
> 

There are still some watches left, probably files cvs removed on some branch or so. Editors, too, I'll attach that in a minute.
If there were watches for other locals too, how come, that only the German builds were affected?
Attached file remaining editors
Please do include a script (or something) in 2.0.0.2 that will fix this issue without having to manually change the permissions
i fixed it now, i've given the write rights to the folder of the profile, it was writeprotectet now it works!
Attachment #250479 - Flags: review?(rhelmer) → review+
Attachment #250479 - Flags: approval1.8.1.2?
Attachment #250479 - Flags: approval1.8.0.10?
benjamin, to solve the problem for 2.0.0.1 German users who now have these files without write permissions on disk, what about this:

audit the callers of nsBrowserDirectoryProvider::GetFile(), and for those who require a writable file (examples:  localstore.rdf, mimeTypes.rdf, search.rdf, panels.rdf, bookmarks.html [and the backups?], microsummary-generators, etc), use nsIFile's permissions and OR with 600. 

Note, some of the callers check for existence, so we'd have to do the OR with 600 at the appropriate place in the callers.

question:  what about symlinks to files?  should we just bail out and forget about ORing if the nsIFile is really a symlink?
Whiteboard: http://www.firefox-browser.de/forum/viewtopic.php?p=336950#336950
Seth, I tend to think we should just fix the problem for those files that are actually affected. And we could perhaps apply the correct permissions in the browserdirprovider or xredirprovider, rather than in all the clients.
benjamin, thanks for the response. 

I'll first confirm (with the 2.0.0.1 de build) that the only affected files are bookmarks.rdf localstore.rdf, mimeTypes.rdf and search.rdf and then I'll work on a patch, per your suggestion and seek a review.
this would take care of mimeTypes.rdf, bookmarks.html, localstore.rdf.  As far as search.rdf goes, I'm not seeing that file being used by firefox.

benjamin, what do you think?
Attachment #250899 - Flags: review?
Attachment #250899 - Flags: review? → review?(benjamin)
Comment on attachment 250479 [details] [diff] [review]
Force permissions on files installed from srcdir, rev. 1

Approved for both branches, a=jay for drivers.  Let's get this landed and get QA some time to verify this.
Attachment #250479 - Flags: approval1.8.1.2?
Attachment #250479 - Flags: approval1.8.1.2+
Attachment #250479 - Flags: approval1.8.0.10?
Attachment #250479 - Flags: approval1.8.0.10+
> As far as search.rdf goes, I'm not seeing that file being used by firefox.

spun that issue off to bug #366442
Attachment #250899 - Flags: review?(benjamin) → review+
landed on the trunk.  this should bake before the branch.

to verify, set permissions (like 400) on mimeTypes.rdf, bookmarks.html, and localstore.rdf in defaults/profile, and then create a new profile.

we should fix the permissions of the files after they are copied to the profile (and once they are used.  for mimeTypes.rdf, you'll need to open the mime types pref dialog.)

cvs ci toolkit/xre/nsXREDirProvider.cpp browser/components/dirprovider/nsBrowser
DirectoryProvider.cpp
Checking in toolkit/xre/nsXREDirProvider.cpp;
/cvsroot/mozilla/toolkit/xre/nsXREDirProvider.cpp,v  <--  nsXREDirProvider.cpp
new revision: 1.50; previous revision: 1.49
done
Checking in browser/components/dirprovider/nsBrowserDirectoryProvider.cpp;
/cvsroot/mozilla/browser/components/dirprovider/nsBrowserDirectoryProvider.cpp,v
  <--  nsBrowserDirectoryProvider.cpp
new revision: 1.13; previous revision: 1.12
done
Did someone figure out *why* this happened in the first place? Sorry if I missed it.
(In reply to comment #55)
> Did someone figure out *why* this happened in the first place? Sorry if I
> missed it.
> 

comment 30.
OK, but there were watches set for other locales as well, why is it, that only the German build was affected?
There were way too many files affected for me to actually look at details, I trust the QA testing here. But apparently not all files were affected.

We can't resurrect that information either, as I don't think that the editors and watches are versioned.
This particular bug is only going to show up with files installed into the profile. Watching files that end up in JARs isn't going to hurt anything. Watching files that are installed but don't end up in the profile could be a problem at the  next update if they are marked readonly and can't be updated.
(In reply to comment #59)
> This particular bug is only going to show up with files installed into the
> profile. Watching files that end up in JARs isn't going to hurt anything.
> Watching files that are installed but don't end up in the profile could be a
> problem at the  next update if they are marked readonly and can't be updated.
> 

Is there anything we can do now to fix any possible update problems?  Anything we can include in the release notes to help those that might run into problems after update?  
rhelmer: ok I saw that but also comment #37 and #41 (#57). Did the DE builds have someone watching something specific that wasn't being watched in the others?
Could we run all installers and do something like
find . \! -perm /200

? I haven't tested that particular line, but that may be worthwhile.
(In reply to comment #62)
> Could we run all installers and do something like
> find . \! -perm /200
> 
> ? I haven't tested that particular line, but that may be worthwhile.

Already done, used this script:
http://lxr.mozilla.org/mozilla/source/testing/release/l10n/verify_l10n.sh

That script is run as part of the release process, so it'll be run after l10n RCs are generated.
(In reply to comment #61)
> rhelmer: ok I saw that but also comment #37 and #41 (#57). Did the DE builds
> have someone watching something specific that wasn't being watched in the
> others?

Running "cvs watch on" causes checkouts to be read-only:
http://ximbiot.com/cvs/manual/cvs-1.11.20/cvs_10.html#SEC89

On the repository side, it manifests as "CVS" metadata dirs in the repository, with "fileattr" files inside containing the list of watchers (if any).

The purpose of this feature is to remind users to run "cvs edit". Oh, and to break firefox builds :)
so the DE builds were the only ones with this type of watching?
Comment on attachment 250899 [details] [diff] [review]
patch (for the trunk)

I'd like this to block the 2.0.0.2 release, but I have not landed on the branch yet.  still need to bake (test and verify) on trunk.
Attachment #250899 - Flags: approval1.8.1.2?
unfortunately, my fix will not solve the entire problem.  my fix addresses the "easy" part:  how to fix the permissions of the user's files.  the "hard" part is how to fix the permssions of the default files, which if not fixed, will prevent complete software updates (and some partial updates.)

as juan found out, if you attempt to install a complete update to a firefox install with a readonly bookmarks.html, updater.exe will fail (even if changes to bookmarks.html are not part of the update).

if a fx 2.0.0.1 user is unable to apply the complete update, they will not be able to get this fix (which would be part of fx 2.0.0.2)

but, a partial update could work, assuming that bookmarks.html (and all other read only files) were not part of the partial update.

from my tests and investigations, I believe this would a solution for the entire problem for the 2.0.0.1 de users who are hitting this bug:

1)

fix updater.cpp, so that before we open with "write only | truncate | create" permissions, we attempt to change permissions on the file and remove the read-only attribute.

2)

take other fixes for 2.0.0.2 (including the "easy" part of this bug), but none that would change the files that were made readonly.  if we take those.

again, we'd have to limit this to a partial update for the 2.0.0.1 de users, as the complete will fail.

now, for the next release, the fixed 2.0.0.2 updater.exe (if we make my proposed change) will be able to apply completes on top of files with read-only attributes, assuming the 2.0.0.2 updater.exe is able to fix the permissions as I've described above.

note, disregarding the "read only" attribute of "application owned" files on windows is something robert strong has been advocating, see bug #322426

robert / bsmedberg, comments?
well there are two types of fx 2.0.0.1 de users out there:

1)  ones you got the "bad" fx 2.0.0.1 de bits
2)  ones who got fx 2.0.0.0 de and upgraded to 2.0.0.1 de (and are ok.)

it is the first type of fx 2.0.0.1 de users who are affected here.
Comment on attachment 250899 [details] [diff] [review]
patch (for the trunk)

>+        rv = file->SetPermissions(permissions | 0644);

r- dveditz: the permissions should be 0600 in both places.

Otherwise, though, we'd like to get this into the branch.
Attachment #250899 - Flags: review-
Comment 68 is saying we need another patch that fixes updater.exe as well, right?
> Comment 68 is saying we need another patch that fixes updater.exe as well,
> right?

yes, and that is being tracked by bug #367084
Comment on attachment 251725 [details] [diff] [review]
patch, ported to the MOZILLA_1_8_BRANCH (with 0600 perms, per dveditz)

seeking r/a for 1.8.1.2 branch from dveditz
Attachment #251725 - Flags: review?(dveditz)
Attachment #251725 - Flags: approval1.8.1.2?
Comment on attachment 251722 [details] [diff] [review]
supplimental patch for the trunk, per dveditz

note, in my local tree I have fixed some line ending problems that I accidentally checked in with my original patch.
an additional change is needed.  

if bookmarks.html was "read only" attribute set, the user might have a bookmarks.html.moztmp with the "read only" attribute set, if they tried to modify bookmarks.

because bookmarks.html.moztmp exists, and has the "read only" attribute set, we're unable to make changes to bookmarks even if we fix bookmarks.html

one way to fix this in nsLocalFileWin.cpp's ::CopySingleFile(), http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileWin.cpp#1447

1437             // the file exists.  Check to make sure it is not a directory,
1438             // then move it out of the way.
1439             if (fileInfo64.type == PR_FILE_FILE)
1440             {
1441                 backup.Append(destPath);
1442                 backup.Append(L".moztmp");
1443 
1444                 // remove any existing backup file that we may already have.
1445                 // maybe we should be doing some kind of unique naming here,
1446                 // but why bother.
1447                 _wremove(backup.get());
1448 
1449                 // move destination file to backup file
1450                 copyOK = ::MoveFileW(destPath.get(), backup.get());
1451                 if (!copyOK)
1452                 {
1453                     // I guess we can't do the backup copy, so return.
1454                     rv = ConvertWinError(GetLastError());
1455                     return rv;
1456                 }

The other way would be to add code to nsBrowserDirectoryProvider.cpp, to also check if the {file}.moztmp exists, and if so, also fix its permissions.
Comment on attachment 250899 [details] [diff] [review]
patch (for the trunk)

Clearning approval1.8.1.2 request since we have new patch for the branch.
Attachment #250899 - Flags: approval1.8.1.2?
Comment on attachment 251810 [details] [diff] [review]
patch, attempt to make the .moztmp read/write before we remove it.

that last supplimental patch will solve this scenario:

1) user got the "bad" firefox 2.0.0.1 de installer, so bookmarks.html was "read
only" on disk.
2) user ran 2.0.0.1, attempted to modify bookmarks, so we created
bookmarks.html.moztmp, but since we did a copy, it was also "read only".
3) user gets 2.0.0.2 (hopefully, via a partial update) with the fixes in this
bug, so we fix bookmarks.html, but bookmarks.html.moztmp is still readonly, so
we'll fail to be able to modify bookmarks, because we can't remove
bookmarks.html.moztmp.

new patch coming, it should be _S_IREAD instead of S_IREAD.
Attachment #251810 - Flags: review?(dveditz)
Comment on attachment 251845 [details] [diff] [review]
patch, attempt to make the .moztmp read/write before we remove it.

seeking review
Attachment #251845 - Flags: review?(dveditz)
same patch, but with comments and explicit casting of return values to (void) since we ignore them (on purpose).
Attachment #251845 - Attachment is obsolete: true
Attachment #251955 - Flags: review?(dveditz)
Attachment #251845 - Flags: review?(dveditz)
Comment on attachment 251722 [details] [diff] [review]
supplimental patch for the trunk, per dveditz

r/sr=dveditz
Attachment #251722 - Flags: review?(dveditz) → review+
Comment on attachment 251725 [details] [diff] [review]
patch, ported to the MOZILLA_1_8_BRANCH (with 0600 perms, per dveditz)

r=dveditz

Approved for 1.8 branch, a=dveditz

Do we need this code on the 1.8.0 branch also? Didn't QA say DE Linux 1.5.0.9 had the same problem?
Attachment #251725 - Flags: review?(dveditz)
Attachment #251725 - Flags: review+
Attachment #251725 - Flags: approval1.8.1.2?
Attachment #251725 - Flags: approval1.8.1.2+
> Do we need this code on the 1.8.0 branch also? Didn't QA say DE Linux 1.5.0.9
> had the same problem?

I had not heard that.  I'll check with qa@mozilla.org and find out.
Assignee: a.topal → sspitzer
(In reply to comment #86)
> > Do we need this code on the 1.8.0 branch also? Didn't QA say DE Linux 1.5.0.9
> > had the same problem?
> 
> I had not heard that.  I'll check with qa@mozilla.org and find out.


This was repo-wide, so it happened on all branches and the trunk builds (we tested  1.5.0.x and trunk nightlies to confirm)
One side effect of this code is that the handful of folks who may have made these files read-only on purpose to prevent modification will be thwarted. If it's a user with access to admin rights I guess they could change the owner on the file on most OSs as long as they don't run Firefox with admin rights.

I doubt we care for Firefox, but some future XULRunner apps might not be happy with this hack.  Maybe we should only put this on the 1.8 branch(es) where the problem was and leave the trunk unpolluted.
rhelmer's comment #21 seems to imply this is a problem with 1.5.0.9

"1.5.0.9 linux - same problem (win32 installer uses .xpi files so permissions
are not preserved, works as expected)"

checking http://releases.mozilla.org/pub/mozilla.org/firefox/releases/1.5.0.9/linux-i686/de/firefox-1.5.0.9.tar.gz, it is a problem with that tar.gz ball, and we are still mirroring it!

-r--r--r--  1 mozilla mkpasswd 13051 Nov 11  2005 bookmarks.html
drwxr-xr-x+ 2 mozilla mkpasswd     0 Dec  6 22:01 chrome
-r--r--r--  1 mozilla mkpasswd   153 Jul 24  2005 localstore.rdf
-r--r--r--  1 mozilla mkpasswd   287 Jul 24  2005 mimeTypes.rdf
-r--r--r--  1 mozilla mkpasswd  3287 Jul 24  2005 search.rdf

So, we'll need this fix, and other planned fixes to (updater) address this problem on trunk, 1.8 branch, and 1.8.0 branch.

this also means for 1.5.0.10, for de, we will only want to serve the partial, except we don't know the OS (unless we use the user agent.)

should we fix that .tar.gz ball or take it down?
(In reply to comment #89)
> http://releases.mozilla.org/pub/mozilla.org/firefox/releases/1.5.0.9/linux-i686/de/firefox-1.5.0.9.tar.gz,
> it is a problem with that tar.gz ball, and we are still mirroring it!
> 
> should we fix that .tar.gz ball or take it down?
> 

We didn't pull the 1.5.0.9 tar balls because no one really voiced an strong opinion on that at the time and we were busy dealing with the 2.0.0.1 issue.  If people think it is worth pulling those files as well... though a bit late, we should do it.
Flags: blocking1.8.0.10+
jay / rhelmer / preed:  could we unzip and untar the 1.5.0.9 linux de tar ball, fix the perms, and then re-push it?

(I'm not sure that affects the http://releases.mozilla.org/pub/mozilla.org/firefox/releases/1.5.0.9/linux-i686/de/firefox-1.5.0.9.tar.gz.asc file, but it might.)
Status: NEW → ASSIGNED
Comment on attachment 251722 [details] [diff] [review]
supplimental patch for the trunk, per dveditz

supplimental trunk patch landed:

cvs commit: Examining toolkit/xre
cvs commit: Examining browser/components/dirprovider
Checking in toolkit/xre/nsXREDirProvider.cpp;
/cvsroot/mozilla/toolkit/xre/nsXREDirProvider.cpp,v  <--  nsXREDirProvider.cpp
new revision: 1.51; previous revision: 1.50
done
Checking in browser/components/dirprovider/nsBrowserDirectoryProvider.cpp;
/cvsroot/mozilla/browser/components/dirprovider/nsBrowserDirectoryProvider.cpp,v
  <--  nsBrowserDirectoryProvider.cpp
new revision: 1.14; previous revision: 1.13
done
(In reply to comment #88)
> One side effect of this code is that the handful of folks who may have made
> these files read-only on purpose to prevent modification will be thwarted. If
> it's a user with access to admin rights I guess they could change the owner on
> the file on most OSs as long as they don't run Firefox with admin rights.
> 
> I doubt we care for Firefox, but some future XULRunner apps might not be happy
> with this hack.  Maybe we should only put this on the 1.8 branch(es) where the
> problem was and leave the trunk unpolluted.

dan, was that comment in regards to this patch (https://bugzilla.mozilla.org/attachment.cgi?id=251955) or something else?



Comment on attachment 251725 [details] [diff] [review]
patch, ported to the MOZILLA_1_8_BRANCH (with 0600 perms, per dveditz)

fixed on the MOZILLA_1_8_BRANCH:

Checking in toolkit/xre/nsXREDirProvider.cpp;
/cvsroot/mozilla/toolkit/xre/nsXREDirProvider.cpp,v  <--  nsXREDirProvider.cpp
new revision: 1.37.2.6; previous revision: 1.37.2.5
done
Checking in browser/components/dirprovider/nsBrowserDirectoryProvider.cpp;
/cvsroot/mozilla/browser/components/dirprovider/nsBrowserDirectoryProvider.cpp,v
  <--  nsBrowserDirectoryProvider.cpp
new revision: 1.4.2.4; previous revision: 1.4.2.3
done
(In reply to comment #91)
> jay / rhelmer / preed:  could we unzip and untar the 1.5.0.9 linux de tar ball,
> fix the perms, and then re-push it?
> 
> (I'm not sure that affects the
> http://releases.mozilla.org/pub/mozilla.org/firefox/releases/1.5.0.9/linux-i686/de/firefox-1.5.0.9.tar.gz.asc
> file, but it might.)

Let's just pull it, it'd be more confusing to have two different tar.gz files out there (one with fixed perms and one not) with the same version #/build ID.

I'll do this in the morning if there are no objections.
rob, pulling it sounds ok to me.  

Should we also:

1) figure out how many people downloaded that .tar.gz for schrep (who was trying to figure out how many people were affected by the 2.0.0.1 de problem)

2) post readmes like:

http://releases.mozilla.org/pub/mozilla.org/firefox/releases/2.0.0.1/win32/de/readme_en.txt
http://releases.mozilla.org/pub/mozilla.org/firefox/releases/2.0.0.1/win32/de/readme_de.txt
Attached file missed a spot in readme_en (obsolete) —
Attachment #252073 - Attachment is obsolete: true
(In reply to comment #93)
>> I doubt we care for Firefox, but some future XULRunner apps might not be
>> happy with this hack.  Maybe we should only put this on the 1.8 branch(es)
>> where the problem was and leave the trunk unpolluted.
> 
> dan, was that comment in regards to this patch
> (https://bugzilla.mozilla.org/attachment.cgi?id=251955) or something else?

No, the nsXREDirProvider.cpp part of attachment 250889 [details] [diff] [review] (or even all of it). Do we really want that permanently on the trunk? bsmedberg's call, I'm just raising it as a potential issue to make sure it's been thought through.

I don't see a fix for search.rdf (comment 10, 13), or did that turn out not to matter?
Comment on attachment 251955 [details] [diff] [review]
patch, attempt to make the .moztmp read/write before we remove it (with better comments)

r=dveditz
Attachment #251955 - Flags: review?(dveditz)
Attachment #251955 - Flags: review+
Attachment #251955 - Flags: approval1.8.1.2?
Attachment #251955 - Flags: approval1.8.0.10?
(In reply to comment #97)
> Can a German speaker please do the same to
> http://releases.mozilla.org/pub/mozilla.org/firefox/releases/2.0.0.1/linux-i686/de/readme_de.txt
> ?
> 
taking :) will do this
Attached file one more inappropriate "2.0" mention (obsolete) —
thanks for catching that Tomcat.
Attachment #252074 - Attachment is obsolete: true
Attachment #252076 - Attachment is obsolete: true
Attachment #252073 - Attachment is patch: false
Attached file DE-Readme for 1509 (obsolete) —
-> readme_de for 1509

I also added a note that this is not a problem on Windows and Mac, to aviod confusion.
Comment on attachment 251955 [details] [diff] [review]
patch, attempt to make the .moztmp read/write before we remove it (with better comments)

approved for 1.8/1.8.0 branches, a=dveditz for drivers
Attachment #251955 - Flags: approval1.8.1.2?
Attachment #251955 - Flags: approval1.8.1.2+
Attachment #251955 - Flags: approval1.8.0.10?
Attachment #251955 - Flags: approval1.8.0.10+
> No, the nsXREDirProvider.cpp part of attachment 250899 [details] [diff] [review] (or even all of > it). Do we really want that permanently on the trunk? bsmedberg's call, I'm just
> raising it as a potential issue to make sure it's been thought through.

I see your point ("if someone intended to make bookmark.html, localstore.rdf, mimeTypes.rdf readonly on purpose, we'll ignore that"), but I think we do want this change on the trunk.  benjamin, if you disagree that it should be on the trunk, let me know.

> I don't see a fix for search.rdf (comment 10, 13), or did that turn out not to
> matter?

that file is not used, so it doesn't matter here.  but, it does matter for software update.  unused or not, having a readonly search.rdf will cause a complete update to fail, but I have a patch in another bug to address that.

thanks again for the reviews, dan.
Comment on attachment 251955 [details] [diff] [review]
patch, attempt to make the .moztmp read/write before we remove it (with better comments)

fix checked into trunk:

Checking in nsLocalFileWin.cpp;
/cvsroot/mozilla/xpcom/io/nsLocalFileWin.cpp,v  <--  nsLocalFileWin.cpp
new revision: 1.169; previous revision: 1.168
done
same code, same comments, but using the nsWinAPI wrapper.
Comment on attachment 252122 [details] [diff] [review]
attachment #251955 [details] [diff] [review] ported to the MOZILLA_1_8_BRANCH

fixed on the MOZILLA_1_8_BRANCH

carrying over dveditz's r/a for the branch from attachment #251955 [details] [diff] [review]

same comments, same code, but the branch uses the nsWinAPI layer, instead of calling _wchmod() directly (like we do on the trunk.)

Checking in nsLocalFileWin.cpp;
/cvsroot/mozilla/xpcom/io/nsLocalFileWin.cpp,v  <--  nsLocalFileWin.cpp
new revision: 1.144.2.9; previous revision: 1.144.2.8
done
Attachment #252122 - Flags: review+
(In reply to comment #104)
> DE-Readme for 1509
> 
> -> readme_de for 1509

Shouldn't this file be encoded in UTF-8?
Keywords: fixed1.8.1.2
Whiteboard: needs landing on 1.8.0 branch
Firefox 1.5.0.9 for Linux DE has been pulled from FTP, and README files put in it's place. If you have further corrections to the file (e.g. UTF-8 encoding), please post a new version. I'd rather get what we have out there now than wait any longer.
as requested per comment#110 UTF-8 Version of the German Readme
Attachment #252091 - Attachment is obsolete: true
(In reply to comment #112)
> Created an attachment (id=252618) [details]
> ReadMe DE - UTF8 Version
> 
> as requested per comment#110 UTF-8 Version of the German Readme

Uploaded; thanks!
Comment on attachment 251725 [details] [diff] [review]
patch, ported to the MOZILLA_1_8_BRANCH (with 0600 perms, per dveditz)

seeking official approval for this on the MOZILLA_1_8_0_BRANCH.
Attachment #251725 - Flags: approval1.8.0.10?
on MOZILLA_1_8_0_BRANCH, nsLocalFileWin.cpp uses remove() / chmod(), unlike trunk and MOZILLA_1_8_BRANCH.

the 1.5 branch uses carrying over the previous reviews from attachments #251995 (trunk) and #252122 (MOZILLA_1_8_BRANCH).

seeking a= for the MOZILLA_1_8_0_BRANCH
Attachment #253020 - Flags: review+
Attachment #253020 - Flags: approval1.8.0.10?
Comment on attachment 253020 [details] [diff] [review]
attachment #251955 [details] [diff] [review] ported to the MOZILLA_1_8_0_BRANCH

a=dveditz for 1.8.0 branch
Attachment #253020 - Flags: approval1.8.0.10? → approval1.8.0.10+
Comment on attachment 251725 [details] [diff] [review]
patch, ported to the MOZILLA_1_8_BRANCH (with 0600 perms, per dveditz)

a=dveditz for 1.8.0 branch
Attachment #251725 - Flags: approval1.8.0.10? → approval1.8.0.10+
backport landed on the MOZILLA_1_8_0_BRANCH.

Checking in toolkit/xre/nsXREDirProvider.cpp;
/cvsroot/mozilla/toolkit/xre/nsXREDirProvider.cpp,v  <--  nsXREDirProvider.cpp
new revision: 1.37.2.3.2.1; previous revision: 1.37.2.3
done
Checking in browser/components/dirprovider/nsBrowserDirectoryProvider.cpp;
/cvsroot/mozilla/browser/components/dirprovider/nsBrowserDirectoryProvider.cpp,v
  <--  nsBrowserDirectoryProvider.cpp
new revision: 1.4.2.1.4.1; previous revision: 1.4.2.1
done
Checking in nsLocalFileWin.cpp;
/cvsroot/mozilla/xpcom/io/nsLocalFileWin.cpp,v  <--  nsLocalFileWin.cpp
new revision: 1.144.2.1.2.3; previous revision: 1.144.2.1.2.2
done
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Keywords: fixed1.8.0.10
Resolution: --- → FIXED
so. speaking as someone who *has* done things like this in the past, and who has also dealt with corporations who have done things like this in the past:

where's the bug on making the trunk less unfriendly to us? (comment 106 and earlier)
Reopening, and removing the fixed keywords.

Attachment 250479 [details] [diff] didn't land on either of the stable branches, leading to bug 370266.

Rhelmer, could you share some light on why this went through the RC process? Comment 63 mentions you had a test for this to be run during that.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 250479 [details] [diff] [review]
Force permissions on files installed from srcdir, rev. 1

this was never landed.  (thanks to carsten and axel for noticing that).

re-seeking approval for both the MOZILLA_1_8_BRANCH and the MOZILLA_1_8_0_BRANCH
Attachment #250479 - Flags: approval1.8.1.2?
Attachment #250479 - Flags: approval1.8.1.2+
Attachment #250479 - Flags: approval1.8.0.10?
Attachment #250479 - Flags: approval1.8.0.10+
Comment on attachment 250479 [details] [diff] [review]
Force permissions on files installed from srcdir, rev. 1

Approved for both branches, a=jay  Let's make sure to land these changes this time and add the fixed keywords.  Thanks!
Attachment #250479 - Flags: approval1.8.1.2?
Attachment #250479 - Flags: approval1.8.1.2+
Attachment #250479 - Flags: approval1.8.0.10?
Attachment #250479 - Flags: approval1.8.0.10+
Whiteboard: needs landing on 1.8.0 branch → needs landing on 1.8 and 1.8.0 branches
fixed landed on the MOZILLA_1_8_0_BRANCH:

Checking in Makefile.in;
/cvsroot/mozilla/browser/locales/Makefile.in,v  <--  Makefile.in
new revision: 1.25.2.6.2.1; previous revision: 1.25.2.6
done

fixed landed on the MOZILLA_1_8_BRANCH:

Checking in Makefile.in;
/cvsroot/mozilla/browser/locales/Makefile.in,v  <--  Makefile.in
new revision: 1.25.2.17; previous revision: 1.25.2.16
done
Status: REOPENED → RESOLVED
Closed: 18 years ago17 years ago
Resolution: --- → FIXED
Whiteboard: needs landing on 1.8 and 1.8.0 branches
for those following, 1.5 did not have microsummaries, so it does not have this fix:

-	$(INSTALL) $^ $(FINAL_TARGET)/microsummary-generators
+	$(SYSINSTALL) $(IFLAGS1) $^ $(FINAL_TARGET)/microsummary-generators
Seth,

I just tested http://stage.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla1.8-l10n/firefox-2.0.0.2pre.de.win32.installer.exe 
(dated 15-Feb-2007 12:05) and I am still seeing read-only files in localized/defaults/pref:

-r--r--r-- 1 rhelmer rhelmer 3370 2005-07-24 10:52 search.rdf
-r--r--r-- 1 rhelmer rhelmer  369 2005-07-24 10:52 mimeTypes.rdf
-r--r--r-- 1 rhelmer rhelmer  158 2005-07-24 10:52 localstore.rdf
-r--r--r-- 1 rhelmer rhelmer 7164 2006-09-28 10:06 bookmarks.html

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
rob, can you locate the build log associated with that installer?
Looks like Linux works:
http://stage.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla1.8-l10n/firefox-2.0.0.2pre.de.linux-i686.tar.gz

The problem seems to be that nsinstall -m 644 does not remove the read-only attribute on windows (thanks to seth and preed for tracking that down!)
tomcat points out that l10n/de/browser/searchplugins/* are also read only on win32.

as far as the end user is concerned, this is not a problem (like it is if localstore.rdf is read only)

this could be a problem for software update for 2.0.0.1 -> 2.0.0.2, and it's another reason (like a read only localstore.rdf) why we can't do complete updates for 2.0.0.2 de linux.

I checked, and theses files don't appear to have changed since 2.0.0.1, so we are still ok to offer partial update to 2.0.0.1 de win32 users.

after 2.0.0.2, updater.exe can handle read only files on update.

rhelmer is usnig rsync and looking at CVS/fileattr files to see if any other read only files are lurking.
"cvs watch off" has been run on the trunk, MOZILLA_1_8_BRANCH and MOZILLA_1_8_0_BRANCH (although watches are repository-wide, the appropriate file must be in your workspace for the "cvs watch" command to work").

Axel, *please* advise all localizers to not use this "feature" :) We're looking into a permanent fix on the build system side, but this should let us get this release out.

Long-term, the build system should enforce that the permissions are set as desired; whether this means fixing nsinstall or something else is to be determined.
no more read-only files on /default/profiles and searchplugins/ fixed on Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.0.10) Gecko/20070216 Firefox/1.5.0.10 Mnenhy/0.7.5.0 ID:2007021518 (RC4) 
marking this fixed - will verify this bug for 1.8.1.2 and 1.8.0.10 when we have signed builds
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
Verified fixed for 1.8.1.2 and 1.8.0.10 with Windows 1.8.1.2 RC4 Sign-test / 1.8.1.2 Linux Build also on 1.8.0.10 Linux RC 2 Build
Seems that the solution in here cause bug 364599. Can somebody please re-wrote the solution so user is possible to protect their localstore.rdf? Or give me a reason why we are not supposed to do that.
reply
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: