Closed Bug 21487 Opened 21 years ago Closed 20 years ago

Copy link location copies garbage

Categories

(Core Graveyard :: Tracking, defect, P3)

x86
Linux
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: andyelf, Assigned: mikepinkerton)

References

Details

(Whiteboard: fix in hand, testing, waiting for review from akkana)

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:
Assignee: chofmann → beppe
beppe or trudelle?
Assignee: beppe → buster
Target Milestone: M13
it's actually goes to buster
Status: NEW → ASSIGNED
I don't think this is my bug, but I'll do some investigation.
*** Bug 22383 has been marked as a duplicate of this bug. ***
QA Contact: leger → elig
Copy issue; QA Assigning to self.
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.
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.
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).
*** Bug 22737 has been marked as a duplicate of this bug. ***
Assignee: buster → pinkerton
Status: ASSIGNED → NEW
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?
Maybe i don't understand, but why is this a clipboard problem and not a usage
problem?
Status: NEW → ASSIGNED
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.
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
?
ok, so this isn't my bug. who should it go to?
I thought the current code does |text.length*2| instead of |text.length|, that
needs change, right?
Whiteboard: fix in hand, testing, waiting for review from akkana
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → VERIFIED
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!
Confirm, works fine as of 2000011212 nightly build.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.