Closed Bug 72583 Opened 24 years ago Closed 23 years ago

URLs for images and links should be relative to source page

Categories

(SeaMonkey :: Composer, defect, P3)

Product:
SeaMonkey
Component:
Composer
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.5

People

(Reporter: cmanske, Assigned: cmanske)

References

(
URL
)

Details

(Whiteboard: [behavior],[correctness] EDITORBASE (work done),PDT+)

Attachments

(4 files, 15 obsolete files)

Screen shots of changes to Image Properties dialog.
26.53 KB, image/gif
Details
Updated full patch with change to GetOS() as described above
32.54 KB, patch
cmanske
: review+
kinmoz
: superreview+
Details | Diff | Splinter Review
Fix to compensate for change to nsIOService.idl: "url_Path" should be used instead of "url_Directory" (bug 97983)
594 bytes, patch
Details | Diff | Splinter Review
Additional tweak to full patch: Make enabling of checkbox more robust by leveraging "MakeRelativeUrl()"; fix problem in Color/Background image dialog
1.63 KB, patch
cmanske
: review+
cmanske
: superreview+
Details | Diff | Splinter Review 
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.
OS: Windows NT → All
Hardware: PC → All
pushing to moz1.0
Priority: -- → P3
Target Milestone: --- → mozilla1.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".
Status: NEW → ASSIGNED
Summary: URLs for images should be relative to source page → URLs for images and links should be relative to source page
Target Milestone: mozilla1.0 → mozilla0.9.1
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.
Target Milestone: mozilla0.9.1 → mozilla1.0
pushing this back to 9.2, this will result in broken links when the user pushes 
the file up to a server
Whiteboard: [behavior]
Target Milestone: mozilla1.0 → mozilla0.9.2
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.
Keywords: patch, review
Whiteboard: [behavior] → [behavior] FIX IN HAND need r=, sr=
*** Bug 84140 has been marked as a duplicate of this bug. ***
Not finished yet -- need to solve problem of drive letters in PC URLs
Whiteboard: [behavior] FIX IN HAND need r=, sr= → [behavior]
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.)
Whiteboard: [behavior] → [behavior] FIX IN HAND need r=, sr=
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.
Target Milestone: mozilla0.9.2 → mozilla1.0
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.
Whiteboard: [behavior] FIX IN HAND need r=, sr= → [behavior]
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!
Target Milestone: mozilla1.0 → mozilla0.9.4
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.
Keywords: nsCatFood
Whiteboard: [behavior] → [behavior]FIX IN HAND need r=, sr=
*** 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.

    
  
Keywords: nsbranch+

    
    

    
  
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.


    
  
Depends on: 97679

    
    

    
  
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
Target Milestone: mozilla0.9.4 → mozilla0.9.5

    
  
Blocks: 97852

    
    

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

    
  
Blocks: 32768

    
    

    
  
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.

URL: http://www.jivamedia.com/Test/Test.html

    
    

    
  
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.

Blocks: 53157

    
  

        Attachment #36844 -
        Attachment is obsolete: true

        Attachment #36844 -
        Attachment is patch: false

    
    

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


    
  

        Attachment #37666 -
        Attachment is obsolete: true

    
  

        Attachment #37667 -
        Attachment is obsolete: true

    
  

        Attachment #37697 -
        Attachment is obsolete: true

    
  

        Attachment #45822 -
        Attachment is obsolete: true

    
    

    
  
sr=kin for lines added to:

  EdDialogOverlay.dtd
  EditorImageProperties.dtd

    
  

        Attachment #45823 -
        Attachment is obsolete: true

    
  

        Attachment #47458 -
        Attachment is obsolete: true

    
  

        Attachment #47555 -
        Attachment is obsolete: true

    
  

        Attachment #47605 -
        Attachment is obsolete: true

    
  

        Attachment #47655 -
        Attachment is obsolete: true

    
  

        Attachment #47658 -
        Attachment is obsolete: true

    
  

        Attachment #47663 -
        Attachment is obsolete: true

    
  

        Attachment #47775 -
        Attachment is obsolete: true

    
    

    
  
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
Component: Editor: Core → Editor: Composer

    
    

    
  
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.

Whiteboard: [behavior]FIX IN HAND need r=, sr= → [behavior]FIX IN HAND need sr=

    
    

    
  
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.


    
  

        Attachment #48543 -
        Attachment is obsolete: true

    
  
Blocks: 87921

    
    

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

sr=kin@netscape.com

        Attachment #49148 -
        Flags: superreview+

    
    

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

r=brade

        Attachment #49148 -
        Flags: review+

    
    

    
  
checked into trunk.
Ready to evaluate for checking into 0.9.4 branch
Keywords: patch, 
          
            review
Whiteboard: [behavior]FIX IN HAND need sr= → [behavior]

    
    

    
  
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

        Attachment #50173 -
        Flags: review+

    
    

    
  
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

        Attachment #50173 -
        Attachment is obsolete: true

        Attachment #50173 -
        Flags: review+

    
    

    
  
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

        Attachment #50174 -
        Flags: review+

    
    

    
  
sr=hewitt

    
  

        Attachment #50174 -
        Flags: superreview+

    
    

    
  
Adding correctness Status Whiteboard, correct/expected behavior does not occur.
Whiteboard: [behavior] → [behavior],[correctness]

    
  
Whiteboard: [behavior],[correctness] → [behavior],[correctness] EDITORBASE (work done)

    
    

    
  
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+.
Whiteboard: [behavior],[correctness] EDITORBASE (work done) → [behavior],[correctness] EDITORBASE (work done),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


    
  
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED

    
    

    
  
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.
Status: RESOLVED → VERIFIED

    
    

    
  
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.

Product: Browser → Seamonkey

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: