This was originally filed as part of bug 133083. When publishing images to another directory, I am getting a "user anonymous unknown " message, even though both the image and the HTML file are uploaded. The only other thing is that the url of the image is changed on the HTML file. But that issue is covered in 133083. Steps to Reproduce: 1. Restart NS and launch Composer 2. Create a simple page with two lines of text and one image 3. Click on File - Publish As 4. Fill in all the appropriate information for your FTP server 5. Make sure to check the boxes to "Include images and other files" and "Use this site subdirectory" Type the name of an image directory 6. Click Publish Actual Results: I recieve a message "user anonymous unknown", even though both the HTML file and the image are uploaded. Expected Result: I would expect that I would not get a message since the upload(s) went through fine.

It has now become apparent that Composer is totally screwed by the FTP behavior of assuming anonymous login if username+password is missing or fails. We simply cannot edit FTP urls directly with this problem. In the Publish dialog (Settings Panel) or Publish Settings dialog, the user can supply a HTTP address that is the alias for the FTP address. If that is not supplied, we get "login failed" when publishing files (see bug 133823) This is because after publishing, we change the URL of the page to the ftp address, but without the username+password, and BOTH are needed to successfully load the page. We could insert the "username:password@" into the document URI, but I think that is very evil. You NEVER want the password to show in plain text, of course, like in the window title and many other places that access the document's location.href. So after publishing like this, further attempts to save changes will result in constant "login failed" alerts. So publishing via FTP *really* requires a correct HTTP alias address. You should never be editing an FTP url directly.

Luckily, it turns out that this is mostly caused by bug 134248. The document URL is mangled by StripUsernamePassword (scheme is deleted!), which is causing the "Login incorrect" messages.

I'm finally comming to grips with the intracacies of FTP publishing. To make this all work, we must always include the username in the final document URL when editing a page with an FTP address, thus after publishing with FTP, the document URL should be somthing like: ftp://username@ftp.foo.com/myfile.html If we do that, when the user edits a page, she *will* be prompted for a password, instead of getting the insidious "Login incorrect" alert dialog. And if we store the password correctly in the publishing code, user will not be prompted at all. This has consequences in various parts of the publishing code. E.g.: 1. When trying to find existing Publishing sites in the site database by matching the document URL, we must pay attention to the username (this fixes the problem of supporting multiple FTP users at a site, bug 130737) 2. Password saving and retrieval must be tweaked as the url used is sensitive to whether or not the username is embedded in the url. In addition, password manager urls do NOT include the terminal "/" in its site urls. 3. Realizing that users may enter "fully qualified" urls in the browser to view an FTP page (e.g.: ftp://username:password@ftp.foo.com/myfile.html), and then use "Edit Page" to edit it in Composer, we have to be very careful to not expose the password in plain text in various places in Composer, such as in the Recent Files menu and in Page Properties. And we certainly should strip out the password when storing a url in recent files history prefs.

*** Bug 130737 has been marked as a duplicate of this bug. ***

This fixes all the problems described above. In composer commands:@@ -1195,7 +1203,10 @@: Updates changed password in publish data after getting a password prompt from FTP. @@ -1556,20 +1581,25 @@ is in the method GetDocUrlFromPublishData() that figures out the document url from the publishing data Changes to editor.js are for the "Recent Files" menu. Changes to publishprefs.js fixes all problems with password handling. It's more changes then I'd like, but to do what was needed was making things too complicated, so instead I replaced the very funky method "GetMatchingPublishUrlLength()" with the much more understandable "FillInMatchingPublishData()" [Brade will really like that :)] This simplifies and cleans up code relating to finding publish data saved in prefs given a document url.

Comment on attachment 76918 [details] [diff] [review] Patch v1 >Index: ComposerCommands.js >=================================================================== >RCS file: /cvsroot/mozilla/editor/ui/composer/content/ComposerCommands.js,v >retrieving revision 1.113 >diff -u -r1.113 ComposerCommands.js >--- ComposerCommands.js 27 Mar 2002 03:45:49 -0000 1.113 >+++ ComposerCommands.js 31 Mar 2002 00:44:45 -0000 >+ if (!ret) >+ setTimeout("CancelPublishing()",0); do: |setTimeout(CancelPublishing, 0);| instead I didn't see anything else on a quick look trhough, but I'm not likely to have time to apply and test until the middle of next week.

I need to discuss this patch with Charley. I don't see where "InsertUsernameIntoUrl" is. Also, I don't think the issue of showing people's passwords in various places should be buried in this bug (since it's such an important issue).

InsertUsernameIntoUrl() was already checked in. Changes for "setTimeout" were done as suggested.

Comment on attachment 76918 [details] [diff] [review] Patch v1 r=brade for the changes in the patch in ComposerCommands.js (with bbaetz' suggestion) and publishprefs.js These should be in a separate bug to track the issue: * EdPageProps.js * editor.js I worry that these changes are incomplete

The "strip password from urls" issue is now in bug 134695, so please ignore changes to editor.js and EdPageProps.js in "Patch v1"

This bug is critical to Publishing feature

- if (ret && gPublishData) + if (!ret) + setTimeout("CancelPublishing()",0); + + if (gPublishData) UpdateUsernamePasswordFromPrompt(gPublishData, gPublishData.username, pwObj.value, saveCheck.value); In the old code, you only executed UpdateUsernamePasswordFromPrompt() when |ret| was non-zero, now you don't check it before calling it. Intentional? FindSiteIndexAndDocDir() now calls FillInMatchingPublishData() in a loop, is there a potential that the list we are looking through is long? I ask because the passed in docURL won't change between each iteration of the loop yet, each call to FillInMatchingPublishData() will do the same work to split the docURL into pieces and strip out the password. I'm just wondering if it's more efficient to restructure some of the code to do this work once?

Kin: To be completely correct, yes the new line should be: + if (ret && gPublishData) UpdateUsernamePasswordFromPrompt(gPublishData, gPublishData.username, pwObj.value, saveCheck.value); although it doesn't really matter since ret = false means user canceled publishing and we will delete gPublishData in CancelPublish(), so the effect of UpdateUsernamePasswordFromPrompt is ignored. I'll make that change.

On the issue of: "FindSiteIndexAndDocDir() now calls FillInMatchingPublishData() in a loop" Yes, this is less efficient. No, I don't expect these lists to be very long; it's the number of successfully-published directories within a publishing site. IMHO, the method FillInMatchingPublishData() is so much better than the obtuse GetMatchingPublishUrlLength() that the benefit of that clarity outways the performance hit. Brade: Don't you agree?

Oops! I was wrong in last comment: The number items in list (number of times FillInMatchingPublishData() is called) is the number of publishing sites in the prefs, not the number of subdirectories. I'd expect the former to be even less than the latter, since the code tries very hard to keep the number of sites down by using a separate list of subdirectoriest for each site.

Comment on attachment 76918 [details] [diff] [review] Patch v1 sr=kin@netscape.com with the |ret| change mentioned above, and with the assumption that as you suggest there is no significant perf penalty to worry about.

adt1.0.0+ (on behalf of ADT) for checkin to the 1.0 trunk.

As of the 04-02 trunk build, I am not recieving the original "user anonymous unknown" message. But it looks like this patch will fix much more than just the original problem.

Comment on attachment 76918 [details] [diff] [review] Patch v1 a=asa (on behalf of drivers) for checkin to the 1.0 trunk

checked in

Verified on 04-05 trunk.

*** Bug 136430 has been marked as a duplicate of this bug. ***

*** Bug 136431 has been marked as a duplicate of this bug. ***