"copy link location" fails to copy the letter H and the number 0 where present

RESOLVED FIXED in mozilla2.0b12

Status

()

Core
Layout
--
minor
RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: joe vella, Assigned: Rich Walsh)

Tracking

({fixed1.9.0.20})

Trunk
mozilla2.0b12
x86
All
fixed1.9.0.20
Points:
---

Firefox Tracking Flags

(status1.9.2 .17-fixed, status1.9.1 .19-fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9a2) Gecko/20070215 Minefield/3.0a2
Build Identifier: Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9a2) Gecko/20070215 Minefield/3.0a2

using "copy link location"  omits the letter h and the number 0 so that the pasted link looks like this: ttp:/ow_do_you_do.tml   This seems to happen after the application has been in use for some time.  Once it begins, it is repeatable every time.  Exiting firefox and reloading it will restore proper behaviour.

Reproducible: Always

Steps to Reproduce:
1. right click on a link
2. pick 'copy link location'
3. paste the result somewhere - such as the 'actual results' box below..
Actual Results:  
ttps://bugzilla.mozilla.org/sow_bug.cgi?id=299482

Expected Results:  
https://bugzilla.mozilla.org/show_bug.cgi?id=299482

Although I'm using minefield at the moment - I have the identical issue with Firefox 2.0.03
This works for me on Mac OS X, Windows, and Linux. Maybe it's OS/2 specific?

Comment 2

10 years ago
Yes, I think it is OS/2 specific. I have seen something like this in the past, too, although never on branch (FF 2.0.0.x) and on trunk we are still broken, so I cannot currently confirm the bug as such.  If the fault is still there, it's probably somewhere in widget...

(I'm also a bit confused by joe's user agent string, the last usable FF nightly that I see is 20070206.)
Assignee: nobody → mozilla
Component: OS Integration → Widget: OS/2
Product: Firefox → Core
QA Contact: os.integration → os2
Hardware: Other → PC
Version: unspecified → Trunk

Comment 3

10 years ago
Maybe this is related. In 1.8.1.2 OS/2 nightly SM when I copy a link location from the context menu of the history window and paste it into an FC/2 edit session, the actual URL has an 0Dh appended to it. If I paste into an empty e.exe session and save, the file ends with 0Dh, 0Ah (saving an empty session results in a 0 byte file). If I paste into an empty epm.exe session and save, the file ends with 0Dh, 0Dh, 0Ah (saving an empty session results in a 2 byte file ending in 0Dh 0Ah). So far I haven't noticed any characters omitted from the paste.

Comment 4

10 years ago
I think the line endings are more of a speciality of the editors than the text copy properties of Mozilla code. I just tried e.exe ae.exe and epm.exe and got different results in all of them (only AE doesn't append anything!).

I also used SeaMonkey trunk -- where this had happened for me quite often some while ago -- extensively over the last weeks and haven't seen the missing-chars problem any more. Neither with missing 'h', '0', or any other character...

Comment 5

10 years ago
Joe, have you seen this again? Perhaps you have tried the more recent nightly builds from <http://sourceforge.net/project/showfiles.php?group_id=183811&package_id=213528>? I haven't seen this at all any more, so I would tend to resolve this as WFM.
(Reporter)

Comment 6

10 years ago
I Hadn't seen it for a while, then got it again recently 0- I'll try the new build.  thanks.

Comment 7

9 years ago
Have seen it again myself in the meantime, but neither with 'h' nor with '0'. instead 'l' and 'i' were missing. And when trying a debug build to determine what the problem was, I couldn't reproduce it any more. In any case, it doesn't happen often --> severity=minor.
Severity: normal → minor
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 8

8 years ago
It happens quite often here with FF Minefield/3.6a1pre on os/2.

Comment 9

8 years ago
If you can reliably reproduce this (even after restart of FF and system restart?!), I can create a debug DLL with some additional output. But then I need to know the _exact_ build you are running (please tell me the full BuildID from Help -> About).

Comment 10

8 years ago
Looking at the code in nsClipboard.cpp I wonder why we never added the OBJ_ANY flag to the DosAllocSharedMem() calls in that file while we did so in all the other files when we added high-memory support. Does the PM clipboard not work with himem?

Comment 11

8 years ago
Replying to Comment #9
I can reproduce this after restarting Firefox, if you can give me some directions to produce the debug DLL, I can look at it.
(Assignee)

Comment 12

8 years ago
So far, I've found that the unicode text is delivered to the OS/2 clipboard code without the missing characters (yesterday I was losing 'p' & '9' - today, it's 'p' & '?').  We'll have to look upstream to see where they're disappearing.

As to hi-mem for the clipboard:  don't.  The data has to be in the lower 512 to be accessible to 16-bit code, most notably, VIO windows.  In any case, one 64k segment just isn't that big a deal.

Comment 13

8 years ago
I don't know how to explain this but since July it has not happened for me again. And I haven't seen any complaints on the newsgroup since then. This seems to come up again every few months, but then vanish again soon.

What should we do with this?

Comment 14

8 years ago
In 3.5 it was impossible to use the "copy link location" menu because a lot of characters where missing *every* time.

Now with 3.6b1 all "p" are missing.

Comment 15

8 years ago
(in reply to comment #13)
I just tested on a trunk Firefox pulled a couple of days ago. d and p at least were missing.
Don't have the problem with trunk built Seamonkey though.

Comment 16

8 years ago
Just to add to the list, I am seeing "p" , but NOT "d" missing in 3.6b1.

Comment 17

8 years ago
Is there any way how we could debug this?
Is there a tool showing the contents of the clipboard?

Any idea why it depends on the letter? It would be normal if letters randomly disappear, but it is a bit strange that only specific letters disappear.

Comment 18

8 years ago
I notice this bug has spread to Seamonkey, Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9.3a1pre) Gecko/20091202 SeaMonkey/2.1a1pre, with periods not being copied to the clipboard from an URL.
Interestingly copying an email address works fine.

Comment 19

8 years ago
In my case the ONLY place I see it happening is when I use the context menu item "Copy Link Location". The other copy functions seem to work fine. Why it would choose that one is beyond me.

Comment 20

8 years ago
(In reply to comment #19)
I meant to add that this is from the right click context menu, "Copy Link Location", and "Copy Email Address" which I'd assume use similar code to access the clipboard.

Comment 21

7 years ago
Been using Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9.2.3) Gecko/20100403 Firefox/3.6.3 for several days now. Seems to be pretty stable.   
However whenever I select "Copy Link location" to copy a URL, the 
resulting copied link is missing some characters in the URL.

"tt://ownoa.smeey.info/ostgresq-..3-os2-200006a.zi"

which should be

"http://download.smedley.info/postgresql-8.4.3-os2-20100406a.zip"

Steve Wendt pointed me to this bug.  What is weird is that this occurs on both my T42p and my T61 and it is the exact same characters that are missing, so it is not random.  This is the first I am seeing this problem.  Just to let people know it is still occurring.  I don't see any issue with the "Copy Email Address" function.

Comment 22

7 years ago
Same thing here on FF3.6.3. FF 3.5.9 is ok. Both on eCS 2.0RC7a, using the same profile and plugins. It seems to be worse than it was back in FF 2.x.x.x, i.e. losing more letters (H,h,D,d,p,x,S,V 1).

From 3.6.3

tt://www.wunergroun.com/raar/miecomosite.as?region=c3&size=2&I=9

From 3.5.9

http://www.wunderground.com/radar/mixedcomposite.asp?region=c3&size=2x&ID=SHV19
(In reply to comment #10)
> Looking at the code in nsClipboard.cpp I wonder why we never added the OBJ_ANY
> flag to the DosAllocSharedMem() calls in that file while we did so in all the
> other files when we added high-memory support. Does the PM clipboard not work
> with himem?

When first looking into highmem support I had put OBJ_ANY into the clipboard function but found as Rich already pointed out that it didn't play well with VIO.  I could paste into most other applications and but not a command prompt.  I found that I could paste into some other application and then I could paste to a command prompt but not directly into a command prompt.  Deciding that was not optimal I removed it :-)

Updated

7 years ago
Duplicate of this bug: 598365

Comment 25

7 years ago
It's been a problem since FF 3.5.xx with the exception of 3.5.9 for me. 3.6.x loses the "t" and now 4.0b7pre loses "a", "p" and maybe others. I know this has been going on for a long time with no resolution. This bug is marked as "minor", and in some way it is, but I find it frustrating in V4.0b because a "mouse over" a link doesn't show the URL of the link in the bottom bar of the window any more. Being able to copy the link location would be at least somewhat helpful in this situation.

Is there any hope of a resolution?

Comment 26

7 years ago
If the same problem happens or not if copying instead out of history maybe there's a clue to be found there.

Comment 27

7 years ago
I switched to 2.1b1 from Peter's 1.1.18 a week ago. I frequently loose numeral "0", never anything else that I've noticed. Is anyone besides me who sees this using something other than WorkPlace Sans for Gecko 1.9.x system fonts? I'm using Trebuchet MS @10pt with 120 DPI on 1400x1050 set in via CONFIG.SYS as follows:

SET SDDFONTSIZE=medium
REM SET SDDFONTDPI=120
SET SDDICONS=small

Comment 28

7 years ago
I am pretty sure that the characters getting lost has nothing to do with displaying them (fonts, SDD settings, etc.). The one time I debugged it, they already arrived lost in the OS/2 code in nsClipboard.cpp, IIRC.

No idea why I never followed up on comment 11. I think one should start by adding lots of printf()s in nsClipboard::SetClipboardData. Just dumping the bytes of pMozData to the screen could be a first step, like (untested)
   printf("%d bytes in clipboard data\n", NumOfBytes);
   for (int i = 0; i < NumOfBytes; i++) {
     printf("%d: %c %x\n", i+1, (char)pMozData[i], pMozData[i]);
   }

If that is already missing chars, add similar stuff in nsTransferable::GetTransferData or even further up.
(Assignee)

Comment 29

7 years ago
(In reply to comment #28)
> The one time I debugged it[...]

I just finished a debugging session for this problem then read these new comments.  Problem is, I can't get my debug version to mess up.  OTOH, my non-debug build consistently drops 'h' and 'e'.  We're working on it...
(Assignee)

Comment 30

7 years ago
Created attachment 487142 [details] [diff] [review]
fixes "Copy Link Location" bug

This should fix the problem.
Assignee: mozilla → dragtext
Status: NEW → ASSIGNED

Comment 31

7 years ago
Should not all the control characters be filtered out? Seems none should ever be in a link.

Comment 32

7 years ago
Stripping also the \0 fixed the problem?
(Assignee)

Comment 33

7 years ago
Created attachment 490415 [details] [diff] [review]
fixes "Copy Link Location" bug - v2

This update makes the nature of the fix more obvious (i.e. the string wasn't null-terminated).
Attachment #487142 - Attachment is obsolete: true
Attachment #490415 - Flags: review?(wuno)

Updated

7 years ago
Attachment #490415 - Flags: review?(wuno) → review+

Comment 34

7 years ago
This bug was os/2 only? Why?

Comment 35

6 years ago
(In reply to comment #33)
> Created attachment 490415 [details] [diff] [review]
> fixes "Copy Link Location" bug - v2
> 
> This update makes the nature of the fix more obvious (i.e. the string wasn't
> null-terminated).

The solution provided by your patch was also suggested in bug77522#c20. Unfortunately the lines making us trouble were checked in in bug77522
Blocks: 77522

Updated

6 years ago
Attachment #490415 - Flags: review?(bzbarsky)

Updated

6 years ago
Assignee: dragtext → nobody
Component: Widget: OS/2 → Layout
QA Contact: os2 → layout

Updated

6 years ago
Assignee: nobody → dragtext
Comment on attachment 490415 [details] [diff] [review]
fixes "Copy Link Location" bug - v2

r=me.  Thanks for catching this!

I think we should take this for 2.0.  The risk is minimal, and randomly copying the wrong text for links (and also randomly getting incorrect link locations in a11y code, which _would_ be hit by Firefox) seems bad.
Attachment #490415 - Flags: review?(bzbarsky)
Attachment #490415 - Flags: review+
Attachment #490415 - Flags: approval2.0?
Comment on attachment 490415 [details] [diff] [review]
fixes "Copy Link Location" bug - v2

a2.0=dbaron
Attachment #490415 - Flags: approval2.0? → approval2.0+

Updated

6 years ago
Keywords: checkin-needed

Comment 38

6 years ago
Can someone explain why
static char strippedChars[] = {'\t','\r','\n'};
caused the bug and 
static const char strippedChars[] = "\t\r\n";
does not?

And why is it os/2 specific?
Because the former is missing the null-terminator, and therefore the string includes whatever random data follows that memory, up to the next null byte.
(Assignee)

Comment 40

6 years ago
(In reply to comment #38)

> And why is it os/2 specific?

It isn't.  It just happened to manifest itself on OS/2 where the gcc compiler chose to put other random data (including characters found in URLs) next to these 3 bytes.  On another platform, gcc may have put null characters after them (for whatever reason), thus hiding the bug.  Simply changing the data used by the source file could have caused the problem to appear on other platforms - or to disappear on OS/2.

Updated

6 years ago
OS: OS/2 → All
http://hg.mozilla.org/mozilla-central/rev/30d1de20ece2
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12

Comment 42

6 years ago
Crazy. Thanks!

Comment 43

6 years ago
Comment on attachment 490415 [details] [diff] [review]
fixes "Copy Link Location" bug - v2

This bug was introduced long ago by a faulty patch. Although it was most obvious on OS/2 it may have been spurious on other platforms. It should be considered to take it for all branches that are still developed. For the risk assessment see bz's comment https://bugzilla.mozilla.org/show_bug.cgi?id=377392#c36
Attachment #490415 - Flags: approval1.9.2.15?
Attachment #490415 - Flags: approval1.9.1.18?
Attachment #490415 - Flags: approval1.9.0.next?

Comment 44

6 years ago
Talking about risk: can this bug be used to cause a buffer overrun in firefox by constructing a malicious link?
No.
More precisely, this bug could cause Firefox to read memory it doesn't own in the unlikely event that there are no null bytes between the strippedChars address and the next memory not owned by Firefox.  But this would be dependent on the binary layout only, and would happen on any link, independent of the link text.
Comment on attachment 490415 [details] [diff] [review]
fixes "Copy Link Location" bug - v2

Approved for 1.9.2.15 and 1.9.1.18, a=dveditz for release-drivers
Attachment #490415 - Flags: approval1.9.2.15?
Attachment #490415 - Flags: approval1.9.2.15+
Attachment #490415 - Flags: approval1.9.1.18?
Attachment #490415 - Flags: approval1.9.1.18+
Attachment #490415 - Flags: approval1.9.0.next?
Attachment #490415 - Flags: approval1.9.0.next+

Comment 48

6 years ago
Created attachment 514618 [details] [diff] [review]
cvs'ified version of the patch for 1.9.0.next

Updated

6 years ago
Keywords: checkin-needed
Whiteboard: att 490415 to 1.9.2.15 and 1.9.1.18, att 514618 to 1.9.0.x
Landed attachment 514618 [details] [diff] [review] on CVS trunk for 1.9.0.next:

Checking in layout/base/nsPresShell.cpp;
/cvsroot/mozilla/layout/base/nsPresShell.cpp,v  <--  nsPresShell.cpp
new revision: 3.1129; previous revision: 3.1128
done
Keywords: fixed1.9.0.20
Whiteboard: att 490415 to 1.9.2.15 and 1.9.1.18, att 514618 to 1.9.0.x → att 490415 to 1.9.2.15 and 1.9.1.18
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/151e611fc58b
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d9a42eb3a7a5
status1.9.1: --- → .18-fixed
status1.9.2: --- → .15-fixed
Keywords: checkin-needed
Whiteboard: att 490415 to 1.9.2.15 and 1.9.1.18
The "3.6.15" we're releasing today does not fix this bug, the release containing this bug fix has been renamed to "3.6.16" and the bugzilla flags will be updated to reflect that soon. Today's release is a re-release of 3.6.14 plus a fix for a bug that prevented many Java applets from starting up.
You need to log in before you can comment on or make changes to this bug.