Closed Bug 432415 Opened 17 years ago Closed 16 years ago

Strip trailing newlines when editor.singleline.pasteNewlines == 2 (Copy/pasting a cell from an xls/ods file adds an extra space at the end (Excel (Microsoft Office) / Calc (Open Office)))

Categories

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

x86
All
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: stifu, Assigned: ted)

References

Details

(Keywords: verified1.9.0.6)

Attachments

(1 file, 2 obsolete files)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050506 Minefield/3.0pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050506 Minefield/3.0pre Copying and pasting a cell from Excel or Calc to Firefox results in an extra space added at the end of the copied data. To reproduce this, make sure to copy the cell as a whole, not just the text inside of it. Then paste it in any Firefox field (search bar, site fields...). Note that this doesn't work in the URL bar, though (that is to say the URL bar doesn't have this bug, as no extra space is added after the pasted text). If you copy a blank cell, than a single space will be pasted. This doesn't happen with IE. Reproducible: Always Steps to Reproduce: 1. Open/create an Excel/Calc document 2. Copy a cell (that contains text or not) 3. Paste it in any Firefox field (except URL bar) Actual Results: The cell content is pasted with an extra space at its end Expected Results: The cell content is pasted with no extra space
confirmed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008050506 Firefox/3.0.0.0 ID:2008050506, MS Excel 2003 and MSIE7
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows Vista → All
Product: Firefox → Core
Version: unspecified → Trunk
Component: General → Widget: Win32
QA Contact: general → win32
By the way, I just noticed this is a regression. It works fine in Firefox 2. I think that bug can be rather problematic, as I almost got an account temporarily locked because of it. It was on the Dell site, where you have limited amount of chances to input the right password. By having copied and pasted the password from an Excel file, an extra space was appended to the password, making it incorrect.
Keywords: regression
confirmed on Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9) Gecko/2008051206 Firefox/3.0 This bug is unacceptable for people who store passwords in Excel.
the same issue has new Opera 9.5, except it creates TWO spaces after pasted word.
confirmed on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0 I have been using Excel 2003 sp3 and I am experiencing the exact same thing, copy a cell in excel and paste into an field in Firefox 3 results in an added space at the end of whatever was being copied this bug does not exsist in either IE7 or Firefox 2
xref bug 85649 newline POP3 password -- which needs some QA love
re: MS WORD The addition of ASCII 32 during the paste function can also be simulated when copying text from MS Word. Using MS Word cut both a LF AND a CR at the end of the text string (or a CR AND a LF). Using this MS Word test I get 2 ASCII 32 (spaces) using Firefox. Whereas, MS IE 6.0 handles the paste normally (no spaces). If you cut just one from MS Word i.e. either one CR or one LF, then Firefox correctly will ignore that during the paste function. re: MS EXCEL The bug is very consistent. Therefore I have learned that the temporary work-around for User ID's and passwords stored in Excel - is to [backspace] exactly one character after each paste. Then the user ID and password will be accepted at the web site.
This particular bug is a show-stopper for me, it is preventing me from upgrading from firefox 2 to 3. I'm just a regular user so all I can do is just announce that there are some people who can't use firefox 3 because of this bug. It's not passwords that I copy, but a big part of my job is to enter existing data (from excel) into one EU-project's database through a web portal consisting of web forms. It will take months before I'm finished and I'm looking at 700 items each consisting of about 20 cells of data, so one more keypress to 14000 copy-paste operations is not only a lot of work but also generates a huge opportunity for error. ps. I know it would all be avoided with a mass import of data through a format conversion, but we have begged for that for a year now and it just is not going to happen. So please fix this one! I decided to comment this after I got a bad virus infection on friday through the latest 2.x version and had to restore entire system partition.
I have 30 years working with PCs. This extra space is very annoying. My whole department pastes part numbers on intranet programs all the time. Now we have an extra-space that generates an error. Many other uses for the copy-and-paste are also affected. PLEASE!!! Give some time to fix this error A.S.A.P. !!! Thanks
Any chance of this being fixed for Firefox 3.1? Can we do anything to help, like find a regression range?
Flags: wanted1.9.1?
Jim, any chance you can take a look at this? Thomas, a regression range would be extremely helpful!
Assignee: nobody → jmathies
Flags: wanted1.9.1? → wanted1.9.1+
Priority: -- → P2
(In reply to comment #17) > Jim, any chance you can take a look at this? > > Thomas, a regression range would be extremely helpful! yep!
Ok, so there is a carriage return on the end of these cells when they are pasted. In standard text edits, the current rules require these be converted to spaces. I found this in nsTextEditRules.cpp if your looking for past discussion. // People have lots of different ideas about what text fields // should do with multiline pastes. See bugs 21032, 23485, 23485, 50935. // The six possible options are: // 0. paste newlines intact // 1. paste up to the first newline (default) // 2. replace newlines with spaces // 3. strip newlines // 4. replace with commas // 5. strip newlines and surrounding whitespace // So find out what we're expected to do: The short answer is, this has come up before. The default is 2, but you can change it to 3 using the following pref - editor.singleLine.pasteNewlines That should solve the problem. I think we can close this out.
bug 406062 was where it was added, although i can't find the bug that made the change to text
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → WONTFIX
Jim, Thank you for your solution. I'm sure a lot of people will benefit form this. Just so it was clear for everyone here are the steps to fix the bug: 1. Open file with editor at C:\Program Files\Mozilla Firefox\defaults\pref\firefox.js 2. Search for editor.singleLine.pasteNewlines", 2 3. Replace it with editor.singleLine.pasteNewlines", 3 4. Restart Firefox.
(In reply to comment #21) > Jim, > > Thank you for your solution. I'm sure a lot of people will benefit form this. > Just so it was clear for everyone here are the steps to fix the bug: > 1. Open file with editor at C:\Program Files\Mozilla > Firefox\defaults\pref\firefox.js > 2. Search for editor.singleLine.pasteNewlines", 2 > 3. Replace it with editor.singleLine.pasteNewlines", 3 > 4. Restart Firefox. Or just use 'about:config'! In the text edit, type in 'pasteNewlines', that should filter down to the individual pref.
(In reply to comment #19) > I found this in nsTextEditRules.cpp > // The six possible options are: > // 0. paste newlines intact > // 1. paste up to the first newline (default) > // 2. replace newlines with spaces > // 3. strip newlines > // 4. replace with commas > // 5. strip newlines and surrounding whitespace > // So find out what we're expected to do: > The short answer is, this has come up before. The default is 2, but you can > change it to 3 using the following pref - > editor.singleLine.pasteNewlines > That should solve the problem. I think we can close this out. MY COMMENTS: As per the nsTextEditRules.cpp the DEFAULT is supposed to be "1" (not a "2") yet Firefox is "shipping" with a "2." The powers that be should be informed to change the "2" to a "1" I changed my firefox.js to: editor.singleLine.pasteNewlines", 1 this way I conform to the proper default. pasting within Firefox now works great!!
The platform and Firefox have different defaults. It's just the way it is. Thunderbird sets a different default as well: http://mxr.mozilla.org/comm-central/source/mail/app/profile/all-thunderbird.js#211 We could add some special-casing here to strip a trailing newline before converting them to spaces. It's probably not useful to put a space in in that case. In general however, we think it's more useful to paste all the text you have on the clipboard instead of cutting it off at the first newline, which is why we flipped the pref.
One of the benefits of setting this preference to "2" is that it allows multi-line address cut/paste to work with online mapping applications like Google Maps. Setting the preference to "1" or "3" by default would break this use case. See bug 406062 for a discussion of this. Ted - I like your idea of stripping trailing newlines before converting them to spaces. This would not break Google Maps, and it would seem to address the problems described in this thread.
Morphing this bug to address the problem. This should be a one-line patch, add tString.Trim(CRLF, PR_FALSE, PR_TRUE); right before the call to ReplaceChar here: http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/text/nsTextEditRules.cpp#588
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Summary: Copy/pasting a cell from an xls/ods file adds an extra space at the end (Excel (Microsoft Office) / Calc (Open Office)) → Strip trailing newlines when editor.singleline.pasteNewlines == 2 (Copy/pasting a cell from an xls/ods file adds an extra space at the end (Excel (Microsoft Office) / Calc (Open Office)))
Assignee: jmathies → nobody
Status: REOPENED → NEW
Component: Widget: Win32 → Editor
QA Contact: win32 → editor
Clearing flags since this got morphed!
Flags: wanted1.9.1+
Okay, I'm livid now. After we finally were given a solution on how to undo the "helpful change" some developer did by quiently changing the default settings - by telling users to perform a user-unfriendly hack of the file: firefox.js. -- Tonight an automatic upgrade-download happened and the "helpful change" was made again (just in case I didn't really want to get rid of it the first time, no doubt!) So now users are going to have to remember all their hacks for years to come and continually re-hack after each upgrade? Luckily I remembered how to get to this web page so I could re-read how to do the edits needed... (In reply to comment #23) > (In reply to comment #19) > > I found this in nsTextEditRules.cpp > > // The six possible options are: > > // 0. paste newlines intact > > // 1. paste up to the first newline (default) > > // 2. replace newlines with spaces > > // 3. strip newlines > > // 4. replace with commas > > // 5. strip newlines and surrounding whitespace > > // So find out what we're expected to do: > > The short answer is, this has come up before. The default is 2, but you can > > change it to 3 using the following pref - > > editor.singleLine.pasteNewlines > > That should solve the problem. I think we can close this out. > MY COMMENTS: > As per the nsTextEditRules.cpp the DEFAULT is supposed to be "1" (not a "2") > yet Firefox is "shipping" with a "2." The powers that be should be informed > to change the "2" to a "1" > I changed my firefox.js to: editor.singleLine.pasteNewlines", 1 this way I > conform to the proper default. pasting within Firefox now works great!!
Uh, why are you editing firefox.js? Just open about:config, type in 'pasteNewlines' or something to find it, and change the value. firefox.js is the default prefs; it will always be overwritten. You should be changing your user prefs via about:config.
Thanks you for the head's up. Yes, while "about:config" was mentioned above, I didn't have a clue what that meant. So I edited firefox.js as was also mentioned above. I presume using "about:config" leaves the user preferences unchanged whenever an upgrade is done? If so, then that's what I was looking for. But still pity the poor Firefox users who do not understand that ASCII 32 is being inserted into cut&pastes ;-( And for others like me who were not familiar with "about:config", here's the instruction I found on the internet: Type about:config in the Firefox URL address bar and press Enter, and you'll see all the settings currently enumerated in prefs.js, listed in alphabetical order. Type in "newline" as the filter, double click the item and change the numeric value, as per above.
We believe that this setting is in the best interest of users. I'm sorry that it breaks your use case, we'll try to implement my suggestion from comment 24 so that it works well for all users. Changing this setting was a conscious decision, and this is the only negative aspect I've heard of it, so I don't see any reason we ought to revert the change.
Attached patch simple patch (obsolete) (deleted) — Splinter Review
Here's a simple patch that implements that suggestion.
Assignee: nobody → ted.mielczarek
Blocks: 406062
Attached patch simple patch + test (obsolete) (deleted) — Splinter Review
Add a chrome mochitest.
Attachment #348543 - Attachment is obsolete: true
Attachment #348545 - Flags: superreview?(neil)
Attachment #348545 - Flags: review?(neil)
Ted, I read the patch, and to me that definitely does seem to answer the issues, and actually, is something I had wondered about decades ago as to why that was not the way a cut & paste would work generally, in all applications! After testing and studying cut & paste in back in the 1980's (as a user) I came to the conclusion that there must be good reasons that the powers that be had in mind and just left it at that. Question:: Is this patch up for discussion so that Firefox might use it as a future upgrade? Or is this some user preference? If so, then as a user, I don't know where to start to try to implement it.
If this patch gets review then I intend for it to be in Firefox 3.1, and I might try to get it backported for the next release of Firefox 3.0.x (probably 3.0.6).
Comment on attachment 348545 [details] [diff] [review] simple patch + test > case nsIPlaintextEditor::eNewlinesReplaceWithSpaces: >+ // Strip trailing newlines first so we don't wind up with trailing spaces >+ tString.Trim(CRLF, PR_FALSE, PR_TRUE); It's interesting that eNewlinesPasteToFirst and eNewlinesReplaceWithCommas also strip leading newlines too. (Sadly the test doesn't test that...)
Attachment #348545 - Flags: superreview?(neil)
Attachment #348545 - Flags: superreview+
Attachment #348545 - Flags: review?(neil)
Attachment #348545 - Flags: review+
I can change the test to test those cases as well. Makes sense to have all our assumptions on the table.
Attached patch patch + updated test (deleted) — Splinter Review
Updated the tests to reflect the stripping of leading and trailing newlines in all varieties. Requesting approval for b2. This is a small change that fixes an annoying bug for users of spreadsheets, and it has automated test coverage.
Attachment #348545 - Attachment is obsolete: true
Attachment #348977 - Flags: superreview+
Attachment #348977 - Flags: review+
Attachment #348977 - Flags: approval1.9.1b2?
Attachment #348977 - Flags: approval1.9.1b2? → approval1.9.1b2+
Comment on attachment 348977 [details] [diff] [review] patch + updated test a191b2=beltzner as a deal with the devil to get ted to land a bunch of stuff for me
Status: NEW → RESOLVED
Closed: 16 years ago16 years ago
Resolution: --- → FIXED
Comment on attachment 348977 [details] [diff] [review] patch + updated test Looking for branch approval. This is a very simple change with tests that fixes a small but annoying regression in our copy/paste behavior.
Attachment #348977 - Flags: approval1.9.0.6?
Comment on attachment 348977 [details] [diff] [review] patch + updated test Approved for 1.9.0.6, a=dveditz for release-drivers.
Attachment #348977 - Flags: approval1.9.0.6? → approval1.9.0.6+
Fixed on 1.9.0 branch: Checking in editor/libeditor/text/nsTextEditRules.cpp; /cvsroot/mozilla/editor/libeditor/text/nsTextEditRules.cpp,v <-- nsTextEditRules.cpp new revision: 1.212; previous revision: 1.211 done Checking in toolkit/content/tests/chrome/test_bug253481.xul; /cvsroot/mozilla/toolkit/content/tests/chrome/test_bug253481.xul,v <-- test_bug253481.xul new revision: 1.2; previous revision: 1.1 done
Keywords: fixed1.9.0.6
Verified fixed for 1.9.0.6 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.6pre) Gecko/2009010606 GranParadiso/3.0.6pre and openoffice.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: