When we insert an image via Image Dialog, or set image URL for page background, the URL should be relative to the source page, not an absolute URL. Editing a page on a local disk, then FTP'ing to a web site is extremely difficult because we always use full URL paths, typically "file:///D:/whatever", which is useless once pushed to a server.

pushing to moz1.0

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

A good recipe from sairuh to show problem in links: [follow these steps exactly!]: 1. open a new document in composer. 2. add a link [using Link Editor] in this doc to another file which is in the same directory --and make sure the link is *relative*. eg, "this is a <a href="blah.html">relative link</a>". 3. save the doc. 4. close the doc [exit composer]. 5. reopen the doc in composer. 6. select the line containing the link via shift+end. 7. copy line via ctrl+C. 8. hit enter to go to another line, then paste via ctrl+V. 9. place cursor in the link of the pasted text. 10. click Link Editor button. result: the URL in the Link Location textfield is absolute, instead of relative. instead of seeing "blah.html" i see "file:///u/sairuh/Tests/blah.html".

Editing pages locally, then pushing to a web server is completely busted unless we fix this, so moving back to 0.9.1 for more serious consideration.

charley: i *didn't* have to upload my file to see bug 73687. perhaps it's just a matter of closing composer?

No, it has nothing to do with uploading or saving. We simply always insert the full (absolute) URL for and image or link at the time the attribute is made on an <img>, <a> etc. tag.

Move this to mozilla1.0; Charley has a lot on his 0.9.1 bug list now but I'm sure he'll get to this as soon as he can.

pushing this back to 9.2, this will result in broken links when the user pushes the file up to a server

This fix is reasonably conservative. It only works once the file is saved (and thus has a base URL.) It also works when editing a remote page. We don't try to look through existing page for "href", "src" etc. (that kind of mangling will have to wait for publishing features.) This fix only applies when user inserts or edits links and images using our dialogs, including background images in Page Colors and Background and Composer Editing prefs.

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

Not finished yet -- need to solve problem of drive letters in PC URLs

6/08 patch is more robust than original fix. Uses nsIURI to extract "path" from document href and user's URL strings. Detects drive letter differences in PC paths. Apply primary (10:33) patch in editor/ui/dialogs/content Apply prefs dialog (10:32) patch in editor/ui/composer/content, (although it uses same pattern as link and image dialogs, so reviewes not thoroughly testing can skip that one.)

Ok, so we have to check for Mac drive labels as well. Note that we don't check for other OS's that might be like UNIX (i.e., don't use separate 'drive' name that we must check. But this code will err on the side of *not* making a relative URL. It should never generate a bad relative url by mistaking a drive name for a subdirectory.

Why do we want to store a relative url as a preference? Exactly what does that mean?

ok, I question whether we really want to rush these patches in or not. * The preference patch is wrong. * The user's images and links will be broken if they save as to a new location. * Sometimes users will want a full uri and we don't have a preference or setting or other means to not make it relative. Exactly what is the horrible problem that we are trying to fix here? (note: copy/paste of urls becoming absolute is bug #32768)

Kathy: I don't understand what you're saying about preferences: I'm not storing anything as a preference. The change to the preference dialog is only to make the background URL relative to the page. I agree, we will break links if "Save As" to a new location. The problem we are trying to solve is to be able to edit a web page locally, then ftp that to a server (along with images) to a server, and have the image references work! By using absolute URLs to local files, using Composer to create web pages is near useless. I know, I've been using it! You must edit HTML source to fix all images and links - a real pain. Yes, we will fix all of this with publishing, of course.

How about making it this way: 1. When choosing a file ("Choose file" button) in link properties window, currently the URL we get is absolute. So we would change the behaviour to make a relative URL every time if it is a file in a filesystem that is local to the page. So: if the edited page is on windows on disk E:, we should get relative URLs for files we choose from the same disk, but absolute URLs for files from other disks or network volumes. If the page is on a network storage (NFS, SMB, whatever), we make relative URL's only if the file is on the same network share / network disk / whatever. Example page locations to consider: Windows: G:\homepages\personal\index.htm \\ourserver\mainshare\users\user1\homepage\index.htm (in the second case \\ourserver\mainshare qualify a distinct share) Linux: /home/user1/homepage/index.html (gee, how simple... i love UNIX-alikes...) 2. As to correcting existing pages with absolute URLs, the goal would be to implement a command that would scan the page for URLs and relativise them if they're filesystem URLs. It could be named eg. Edit-> Make links relative. It would present a confirmation dialog: "This command will convert all local filesystem URLs to be relative wherever possible. Continue?" "Yes" "No". 3. If someone wishes to insert absolute URLs for some reason, a pref for always using absolute URLs (as it works now) could be implemented.

We decided to postpone this issues to next version.

Aleksander: Good suggestions, by the way! Just no time to do it now.

I really think that we need the necko folks to implement something to relativise URLs. If we faff around trying to do it ourselves, we'll never get all the edge cases.

Interesting, related docs: UNIX: http://www.perl.com/reference/wrap.cgi?pathconvert http://tamacom.com/pathconvert/ http://tamacom.com/unix/ - search for Path converting library for C language Windows: http://www.freevbcode.com/ShowCode.Asp?ID=2144 also: search for: PathRelativePathTo _fullpath functions in msdn library, Link Tracking: Absolute and Relative Paths article in msdn library (note that this is a counter-example: we shouldn't do it that way, because in some cases there would be annoying and mysterious mis-linkages, eg. where one index.html file is moved with a relative link to another main.html file, and a new main.html file gets in the place of the previous, Mozilla would stubbornly link to the new main.html from index.html instead of the proper main.html)

Some more for Windows: Search MSDN library for: IMoniker::CommonPrefixWith MonikerRelativePathTo IMoniker::RelativePathTo

So, I propose that someone opens a new bug that's assigned to proper product/component for necko guys (what would that be?) and add it to this bug's dependency list.

So the plan is definitely to put robust relativizing code in necko, but for the short term (0.9.4/emojo release), I will add a button to the Image Properties dialog that allows user to relativize a URL -- we won't do it automatically all the time. This will really help ecommerce clients that make a page and are ftp'ing it with images to a web site. Right now, leaving full "file://..." URLs is really annoying!

Patch on 8/14/01 implements the "Make URL Relative to Page Location" feature that is the interim fix until full publishing is available. If page is not saved, the button is still enabled, but the message "Relative URLs can only be used on pages which have been saved" is displayed so the user knows they must save first. This message string already existed for the image map editor, but I changed the ID string to be more general, thus the change to EdImageProps.js to use this new ID. I forgot to include the editor.properties change in the last patch. I'll update shortly.

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

8/28 patch fixes many problems discovered when using relativized URLs: 1. The URL used to load the preview image (and remove it from the image cache) must be absolute. EdDialogCommon.js now contains a set of url-manipulation utilities. Some of this code should be migrated into netwerk interfaces, but this will serve for intitial testing. nsIOService and nsIURL interface methods are used as much as possible (Most notably, a MakeRelativeUrl method is lacking in these interfaces!) 2. The "make relative/absolute" checkbox is implemented in the Image (for SRC), Link (for HREF), and Page Colors and Background (for BACKGROUND). Unfortunately, it cannot be done for the "background" image in the "New Page" prefs panel, since we don't know the correct document URL to use as the base for relativizing. This attribute should be relativized when a document is saved. (That is not part of this task for this bug, but could now be implemented using the utilities provided here.) 3. The state of the checkbox should always reflect the URL in the associated input field. When the user clicks on it, it will attempt to convert the URL to absolute (if checkbox is currently checked) or relative (if it was unchecked) 4. Making a URL relative or absolute is possible only after saving the page. A tooltip when cursor is over the checkbox gives this info to user. 5. Whenever the "Choose File" button is used to select a local file for a URL, we always try to convert to relative URL. Now ready for next round of reviews.

The patch also includes some dialog fixup for EdColorProps.xul, done mostly by Ben Goodger. This was previously reviewed and approved along with a lot of other dialog changes, but not by Composer members, so it was backed out. The changes in this patch are simplify the dialog and I think are a good thing to do.

Directory matching strategy change to searching from beginning of document URL and link/image URL to find common (or shared) directories first. This fixes problems with deep directory trees and simplifies overal logic.

I see this in the latest diff (which seems wrong to me): + if (docScheme == "file" && + os == gUNIX) I expect to see: if (docSheme == "file" && os != gUNIX)

Please check how we handle documents with a <base> tag in the head. Also, please file a new bug on the issue of enabling the anchor functionality which we are disabling to work around bug when document doesn't include filename.

Fixed by the 8/30/01 22:05 patch: 1. Better enabling/disabling of the "Url is relative..." checkbox: both scheme and host must match to convert an absolute into relative url. In current fix, user cannot convert a relative named anchor link into an absolute URL if the base URL doesn't tell us the filename (i.e., it's probably an "index.html" or "index.htm", but we want to avoid creating an invalid URL until we figure out a better solution.) 2. Simplified usage of the "MakeRelativeUrl" checkbox by the Link, Image, and Page Colors dialogs: Only one method needs to be called [SetRelativeCheckbox()] to set checked state and enabling of the checkbox. Generally-usefull utilities GetDocumentBaseUrl(), GetHost(), and GetFilename() methods implemented to support this code. 3. Support for the <base> tag: If page contains "<base href=..." that url is used to relativize instead of the page's actual location, as dictated by W3C standards. Discovered easy to fix bug 97679 in nsIOServices::GetUrlPart() when use to obtain the filename and/or file extension from a URL. That fix is reviewed; waiting for a=.

Some comments for the benefit of PDT when they review this bug for branch checkin A common usage pattern of Composer is to have pages on a local disk which you edit, and then push them to a server. Currently, all page and image links are inserted as absolute urls ("file:///..." ), which are obviously useless once pushed to a server. So with my fix, the default behavior is to always relativize the url obtained when user chooses a local file (most commonly using the "Choose File" button in Link or image dialogs). In these dialogs, there is a checkbox to convert that to absolute or back to relative. ... cmanske54 (12:21:14 PM): This also very important if you are editing remote ("http") pagesand the links or images are pasted into the input fields (most likely theyare absolute URLs), allowing the user to convert to relative by clickingon the checkbox. The state of the checkbox always reflects the state of theURL (any URL having a "scheme" is considered absolute.) So the code gets more complicated by edge issues such as the "volume" or drive names on Mac, PC respectively, when to use case insensitive string comparisons, paying attention to the HTML "base" tag, etc. Those are the issues discovered by testing and reviews which contributed to the delay inchecking this in.

Moving to 0.9.5 per PDT

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

The URL field above points to the base document for testing the features fixed in this bug. Load that URL into the browser for testing the latest patch.

Purpose of the changes to the Image dialog: In the smaller image, the width of the dialog is smaller than it it currently, while giving more room to show the src URL. It is slightly taller because the Advanced Edit button is moved to below the Image Preview window. The larger version when all properties are revealed is exactly the same as current dialog, except the src input is wider to accomodate long URLs. These changes simplify the Image dialog code, eliminating the dual "Advanced Edit" buttons and the dialog's "onMoreFewers()" method altogether (the existing onMoreFewer() method in EdDialogCommon.js is used instead.) This also fixes the bad layout problems in the dialog, like the chopped-off "Choose File" button and other problem like bug 53157. Note that when switching from the smaller to larger modes, both the width and height change, which may seem a bit strange, but makes sense given the amount of new stuff visible in the larger dialog.

r=brade for lines added to EdDialogOverlay.dtd EditorImageProperties.dtd

sr=kin for lines added to: EdDialogOverlay.dtd EditorImageProperties.dtd

I see double slashes when going from relative to absolute urls, but that's apparently coming from bug 84760, nothing Charley can do anything about. I'm slightly concerned about the OS strings -- is it really "X11" for all Unix platforms except Linux? It seems surprising that Linux would be so different. I thought you were going to change the logic so that if it wasn't Mac or Win, it used Unix pathnames? Also, what about OS X (now a primary platform) -- I'm sure that's not going to say X11 or nux, but doesn't OS X also need Unix pathnames? Other than that, it seems to work well and is certainly a nice feature -- we should try to get it in. r=akkana for the js files, for checking into the trunk for further testing, but please address the question of the OS strings asap (we need to be sure to get that right if it's going into the branch).

spam composer change

Right, making sure the OS tests are correct needs to be addressed. This will be much simpler if we get this checked into the trunk.

We explored the various OS strings given by "navigator.platoform" and concluded that using: function GetOS() { if (gOS) return gOS; if (navigator.platform.toLowerCase().indexOf("win") >= 0) gOS = gWin; else if (navigator.platform.toLowerCase().indexOf("mac") >=0) gOS = gMac; else gOS = gUNIX; return gOS; } is the best solution for now.

Comment on attachment 49148 [details] [diff] [review] Updated full patch with change to GetOS() as described above sr=kin@netscape.com

Comment on attachment 49148 [details] [diff] [review] Updated full patch with change to GetOS() as described above r=brade

checked into trunk. Ready to evaluate for checking into 0.9.4 branch

I've already documented this feature in the online help, which was sent out for localization yesterday. Therefore, if this feature *doesn't* make it into the branch, I'll have to remove it from the online help, which will impact the localization effort for the online help.

Patch dated 9/19/01 is checked in. Relativize works now.

Comment on attachment 50173 [details] [diff] [review] Additional tweak to full patch: Make enabling of checkbox more robust by leveraging "MakeRelativeUrl()"; fix problem in Color/Background image dialog r=syd

Comment on attachment 50173 [details] [diff] [review] Additional tweak to full patch: Make enabling of checkbox more robust by leveraging "MakeRelativeUrl()"; fix problem in Color/Background image dialog ignore this. wrong file

Comment on attachment 50174 [details] [diff] [review] Additional tweak to full patch: Make enabling of checkbox more robust by leveraging "MakeRelativeUrl()"; fix problem in Color/Background image dialog r=syd

sr=hewitt

Adding correctness Status Whiteboard, correct/expected behavior does not occur.

I just filed bug 101426. there is an image issue when the relative checkbox is checked when that image and file are on the desktop. see bug 101426

Aside from the behavior Sujay noted in bug: 101426. This is working correctly for me on the Trunk build from 9-24 in Windows 98SE, and Mac OS 9.1. I confirmed this with pages in their own subdirectories as well as on the desktop.

I understand that if this bug does not get fixed it will impact the online Help which is already in translation. We can't afford not to fix this from l10n point of view without impacting our schedule. Please fix this bug.

check it in - PDT+.

Please note that bug 101426 has *NOTHING* to do with this bug. It is a bug found when inserting an image filename that is "relative" (no full pathname) in the image dialog. This new code simply makes it easy to get a relative pathname, but the problem exists in all earlier releases that do not have the code for this bug.

Patch from 9/20/01 checked into 0.9.4 branch

Everything is checked in

Verified on the 9-25 Branch build (0.9.4) using the tests at: http://www.jivamedia.com/Test/Test.html. I tested this on Windows98 SE, and Mac OS 9.1.

I Think, this bug should be markded as closed or duplicate of bug 122227 or thomething like that, beause i don#t belive, that there will not be any mor work on _this_ bug report.