21487 - Copy link location copies garbage

Status: VERIFIED FIXED

(Core Graveyard :: Tracking, defect, P3)

Description

[Andrei Popov]

Overview Description: When copying a link location (right-click on a link in a
browser) trailing garbage is also copied into a clipboard.

Steps to Reproduce:
  1) Go to any page with links
  2) Rigth-click on any link
  3) Select 'Copy
link location'
  4) Go to your favorite text editor and paste content of a clipboard -OR- Try
pasting just copied URL into location URL of Netscape/Mozilla/any other browser

Actual Results:

At the end of an URL there are some garbage characters, just like so:
http://bugzilla.mozilla.org/bug_status.html#rep_platformkH ´oHCÔmË)hiÔôÍ1@

(As you may guess, this is a copy of a link location from a 'Platform' link on
'Enter Bug' page of bugzilla)

Expected Results:

Guess :)

Build Date & Platform Bug Found:
SeaMonkey nightly build 1999120608
Platform: Debian GNU/Linux 2.1 with 2.2.12 kernel on a PPro-200
The same has been the same has been the case with SeaMonkey 1999112008 on the
same platform


Additional Builds and Platforms Tested On:
SeaMonkey 1999112008, same platform, same result

Additional Information:

Comment 1

[chris hofmann]

beppe or trudelle?

Comment 2

[rubydoo123]

it's actually goes to buster

Comment 3

[buster]

I don't think this is my bug, but I'll do some investigation.

Comment 4

[umbrook0]

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

Comment 5

[Eli Goldberg]

Copy issue; QA Assigning to self.

Comment 6

[Akkana Peck]

I glanced at this a little to see if I could help ...

copyLink calls this.copyToClipboard( this.linkURL(), and at this point, linkURL
is fine.

copyToClipboard does, among other things, this:

        transferable.addDataFlavor( "text/plain" );
        // Create wrapper for text.
        var data = createInstance( "component://netscape/supports-string",
                                   "nsISupportsString" );
        data.data = text ;
        transferable.setTransferData( "text/plain", data, text.length * 2 );
        // Put on clipboard.
        clipboard.setData( transferable, null );

Why is it specifying double the length when it's only using text/plain?  It
should either use text/unicode, or use text.length instead of text.length * 2.
Or, more likely, both: the former doesn't work for me if I then request the
selection as plain text, but the latter may not work on non-English systems.

Sdagley seems to be the owner of this code -- CCing him, and Naoki and Frank in
the hope that they might comment on the text vs. unicode issue (are URLs always
8-bit, or can they be more?) as well as myself and the clipboard owner.

Comment 7

[Akkana Peck]

I glanced at this a little to see if I could help ...

copyLink calls this.copyToClipboard( this.linkURL(), and at this point, linkURL
is fine.

copyToClipboard does, among other things, this:

        transferable.addDataFlavor( "text/plain" );
        // Create wrapper for text.
        var data = createInstance( "component://netscape/supports-string",
                                   "nsISupportsString" );
        data.data = text ;
        transferable.setTransferData( "text/plain", data, text.length * 2 );
        // Put on clipboard.
        clipboard.setData( transferable, null );

Why is it specifying double the length when it's only using text/plain?  It
should either use text/unicode, or use text.length instead of text.length * 2.
Or, more likely, both: the former doesn't work for me if I then request the
selection as plain text, but the latter may not work on non-English systems.

Sdagley seems to be the owner of this code -- CCing him, and Naoki and Frank in
the hope that they might comment on the text vs. unicode issue (are URLs always
8-bit, or can they be more?) as well as myself and the clipboard owner.

Comment 8

[nhottanscp]

URL may be in any charset (e.g., Shift_JIS) with escape. If the URL string is
internaly passed around without unescape then text/plain is fine. If we unescape
then we want to use text/unicode and we need charset conversion (from platform
charset to unicode, see nsIPlatformCharset.h).

Comment 9

[Devin Carraway]

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

Comment 10

[buster]

akkana, thanks for the great analysis.
assigning to pinkerton, who I think owns clipboard today.  Pink, if you aren't
the right assignee, can you please pass this off to the right person in your
group?  Maybe sdagley?

Comment 11

[Mike Pinkerton (not reading bugmail)]

Maybe i don't understand, but why is this a clipboard problem and not a usage
problem?

Comment 12

[Mike Pinkerton (not reading bugmail)]

I totall admit my ignorance about encodings, can someone explain our strategy
about encoded text/plain strings? If they are single-byte data with no embedded
nulls, then length is |text.length|, not |text.length*2| regardless of the
encoding, right? Or is this the one big mistake that everyone makes who doesn't
understand i18n? ;)

Is there a good doc to learn about all this stuff? I hate to keep asking the same
dumb questions over and over again.

Comment 13

[nhottanscp]

Yes, you're right, |text.length|, not |text.length*2|. As the length is not used
for the character counts but for the byte length.
About books, how about,
Developing International Software for Windows 95 and Windows NT - Nadine Kano or
CJKV Information Processing - Ken Lunde
?

Comment 14

[Mike Pinkerton (not reading bugmail)]

ok, so this isn't my bug. who should it go to?

Comment 15

[nhottanscp]

I thought the current code does |text.length*2| instead of |text.length|, that
needs change, right?

Comment 16

[Mike Pinkerton (not reading bugmail)]

i think this is fixed (i checked in a fix to the problems described) but i can't
actually dupe the bug on mac/windows and my linux box is dead. feel free to
reopen if it really isn't fixed.

Comment 17

[Eli Goldberg]

This appears fixed using the 2000011110 build on RH 6.0 Linux (also checked
2000011108 Mac OS and 2000011108 Win32).

---

andyelf, might you be open to giving this a quick check, and re-opening it if
you can still reproduce it? Thanks!

Comment 18

[Andrei Popov]

Confirm, works fine as of 2000011212 nightly build.