Closed Bug 377392 Opened 14 years ago Closed 10 years ago

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

Categories

(Core :: Layout, defect)

x86
All
defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla2.0b12
Tracking Status
status1.9.2 --- .17-fixed
status1.9.1 --- .19-fixed

People

(Reporter: joe.vella, Assigned: dragtext)

References

Details

(Keywords: fixed1.9.0.20)

Attachments

(2 files, 1 obsolete file)

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?
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
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.
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...
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.
I Hadn't seen it for a while, then got it again recently 0- I'll try the new build.  thanks.
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
It happens quite often here with FF Minefield/3.6a1pre on os/2.
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).
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?
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.
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.
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?
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.
(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.
Just to add to the list, I am seeing "p" , but NOT "d" missing in 3.6b1.
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.
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.
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.
(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.
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.
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 :-)
Duplicate of this bug: 598365
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?
If the same problem happens or not if copying instead out of history maybe there's a clue to be found there.
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
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.
(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...
Attached patch fixes "Copy Link Location" bug (obsolete) — Splinter Review
This should fix the problem.
Assignee: mozilla → dragtext
Status: NEW → ASSIGNED
Should not all the control characters be filtered out? Seems none should ever be in a link.
Stripping also the \0 fixed the problem?
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)
Attachment #490415 - Flags: review?(wuno) → review+
This bug was os/2 only? Why?
(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
Attachment #490415 - Flags: review?(bzbarsky)
Assignee: dragtext → nobody
Component: Widget: OS/2 → Layout
QA Contact: os2 → layout
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+
Keywords: checkin-needed
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.
(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.
OS: OS/2 → All
http://hg.mozilla.org/mozilla-central/rev/30d1de20ece2
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
Crazy. Thanks!
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?
Talking about risk: can this bug be used to cause a buffer overrun in firefox by constructing a malicious link?
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+
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
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.