57296 - URL in copy and paste does not resolve to absolute URL

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[Frank Tang]

this is similar to the drag and drop case I stated in bug . But this is a copy
and paste issue which have no dependency on drag and drop.

reproduce procedure
1. go to www.mozilla.org
2. select from the upper left corner down to some text so it will include the
mozilla banner image
3. copy
4. open composer
5. paste
expect result
 the part I select paste into composer, the image of the mozilla banner will
show up there
 actual result
everything as expected, but the image does not show up. If you double click on
the broken image icon, you will see the url as "images/mozilla-banner.gif" ,
which is a relative url, it should be resolved into absolute url when it copy. I
believe this is a copy issue not a paste issue.  We should resolve the IMG tag
into absolute URL before we copy. Once the relative URL got copy into clipboard,
it is too late for editor to do anything.

Comment 1

[Frank Tang]

I see this on my 10/16 afternoon window branch pull local build. Also happen on
Mac 10/16 branch build.

Comment 2

[Frank Tang]

Change the summary, I find out it is not just IMG, ANY URL does not get resolved 
into absolute URL when we copy. This is very very bad for editor. Try the 
following
reproduce procedure
1. go to www.mozilla.org
2. select from the upper left corner down to some links.
3. copy
4. open composer
5. paste
6. save that that document.
7. open that document in the browser. 
8. click on one of the link

expect result
 jump to the page that the www.mozilla.org point to.
 actual result
go to a relative url which does not exist on my local drive.

The problem is the copy code need to resolve relative URL into absolute url. 
Otherwise, the composer users will feel very strange. They will think they paset 
some link and later on they will find out that link they paste in point to no 
where. 

Mark it as dogfood. I cannot Use composer to edit my status report. Nobody can 
use composer to do real document editing.

Comment 3

[rubydoo123]

assigning this to brade for the now, Kathy -- how difficult would it be to make
all src attributes absolute?

Comment 4

[Akkana Peck]

This worked in the XIF world, but got regressed by noxif: the code to handle url
absolutizing all got migrated correctly, but wasn't called since we no longer
have to do an encoding step in the clipboard code for html, and it wasn't being
called from the copy encoder either.

Kathy and I worked on a solution to put the code in the copy encoder, and she
gave me this diff, which works for both of us (r=akkana):

Index: nsDocumentEncoder.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/base/src/nsDocumentEncoder.cpp,v
retrieving revision 1.32.4.2
diff -r1.32.4.2 nsDocumentEncoder.cpp
1009,1010c1009,1011
<
<   mFlags = aFlags;
---
>
>   // Make all links absolute when copying
>   mFlags = aFlags | OutputAbsoluteLinks;

Comment 5

[Kathleen :Brade]

this bug has review by a module owner (akkana) but it needs a sr=
Simon can you sr=?

nominate for rtm since this is a dogfood bug as well as a recent regression.

Comment 6

[Simon Fraser [no longer active]]

What are the side-effects of this change? What happens when you want relative 
URLs in your composer document, and you copy and paste within the document; that 
should not absolutize existing relative links.

Comment 7

[Kathleen :Brade]

yes, one of the tradeoffs we have to make for the rtm version is that copy/
pasting a relative url within the same composer document will make it absolute.  
Akkana and I decided that was a better bug to live with in rtm than having 
relative urls copied into Composer or Mail Compose (which will be useless).  

The right fix for the trunk will require more changes which PDT might consider 
too risky for rtm branch.

Comment 8

[Simon Fraser [no longer active]]

It seems like the correct fix for this will be to detect whether the source and 
destination document for the copy are the same (say, isn't this what XIF was 
for?).

But it seems that this change is reverting to pre-XIF behaviour. sr=sfraser for
this fix on the branch. Please keep the bug open, and do a proper fix on the 
trunk.

Comment 9

[Kathleen :Brade]

sfraser--yes, you're right, we'd need to check source/destination locations and 
do the right thing (usually absolute-ize unless it's the same composer window).

set to rtm+ for PDT consideration on the branch.

Comment 10

[Akkana Peck]

There's a separate bug for making links relative again when pasting within the
same composer document.  Pre-noxif, more people felt strongly that always
absolutizing was better than never absolutizing, so that's the behavior we
implemented, with the hope of some day being smarter.

Comment 11

[Phil Peterson]

rtm-. Seems like this trades one set of problems for a different set of
problems, and we don't think either set of problems is serious enough to hold for.

Comment 12

[rubydoo123]

moving to mozilla0.9, PDT deems this not important for rtm

Comment 13

[Kathleen :Brade]

I would like PDT to reconsider taking this fix.

Yesterday I was reminded by QA that broken links don't show up with a broken
link icon as they did in 4.x.  This means that there is no straightforward way
for users to fix broken image urls (broken because they are relative).

Here is a scenario:
  My dad goes to our home page or cnn.com and sees an interesting
article/headline with an image and selects them.  He then copies that selection
and opens a mail compose window and pastes it in the body.  The text appears but
the image doesn't (since most images on these sites seem to be relative urls).
So my dad goes back to the browser window and copies the image again.  Back to
mail compose and paste.  Hmmm... still nothing.  3rd time is a charm? nope.

What really happened is that he pasted the image 3 times but none of the images
are displaying since they are all with a relative url.  It'd be more obvious if
we displayed a broken image icon but that is an unrelated bug which makes this
bug very non-intuitive.

Even if they managed to figure out some way to edit the image url (I don't think
it's possible in mail compose), I doubt most users would be able to make the
relative url absolute.

As I said earlier, this fix is safe and straightforward and will return us to
behavior we had in all of the previous betas (always make all urls absolute).

Comment 14

[Kathleen :Brade]

summary:  patch is in the comments above (added by Akkana for me).
This patch was reviewed by Kin, Akkana and super-reviewed by sfraser.

I think PDT should consider taking this bug because copy/paste of images with
relative URLs won't show up at all (no broken image icon) and users won't know
why they aren't there (bug #41924 and #58646 which is the Composer tracking bug
for the layout bug).

Also, the current behavior is a regression from all previous betas.  The
behavior was broken with the noxif landing because the flag didn't get set for
html copy (only for plain text copy).  The issue Simon (and PDT) have brought up
for maintaining relative URLs in Composer is bug #32768 and has been around for
a long time (with few duplicate bugs or complaints).

Based on the low interest for bug #32768, it seems to me that more users will
run into a problem (or two) with relative URLs pasted into Mail Compose or AIM
than those who explicitly create and then copy urls which are relative in Page
Composer.

Comment 15

[selmer (gone)]

rtm-, the difference between the current and the proposed behaviors is not a 
stop-ship.

Comment 16

[Kristi Fredrickson]

Adding to Composer section of Release Notes

Comment 17

[verah (gone)]

Removing myself from list of cc's. This was release noted.

Comment 18

[Joe Francis]

we have a sep bug for doing the hard part.  lets stop pissing everyone off inthe
meantime.  r=jfrancis

Comment 19

[Kathleen :Brade]

checked in

Comment 20

[Kristi Fredrickson]

Replacing kristif with robinf in the CC: list. Robin Foster-Clark is now handling 
doc issues for Composer.

Comment 21

[James Baxter]

Verified in 8-28 build