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)))

RESOLVED FIXED

Status

()

Core
Editor
P2
normal
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Thomas Goldstein, Assigned: ted)

Tracking

({verified1.9.0.6})

Trunk
x86
All
verified1.9.0.6
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

10 years ago
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
Component: General → General
Ever confirmed: true
OS: Windows Vista → All
Product: Firefox → Core
Version: unspecified → Trunk
Component: General → Widget: Win32
QA Contact: general → win32
(Reporter)

Comment 2

10 years ago
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

Comment 3

10 years ago
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.

Comment 4

10 years ago
the same issue has new Opera 9.5, except it creates TWO spaces after pasted word.

Comment 5

10 years ago
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

Updated

10 years ago
Duplicate of this bug: 439919
Duplicate of this bug: 443953

Comment 8

10 years ago
xref bug 85649 newline POP3 password -- which needs some QA love
Duplicate of this bug: 445132

Updated

10 years ago
Duplicate of this bug: 446667

Comment 11

10 years ago
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.

Comment 12

10 years ago
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.

Comment 13

10 years ago
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

Updated

10 years ago
Duplicate of this bug: 445748
Duplicate of this bug: 459795
(Reporter)

Comment 16

10 years ago
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

Comment 18

10 years ago
(In reply to comment #17)
> Jim, any chance you can take a look at this?
> 
> Thomas, a regression range would be extremely helpful!

yep!
Keywords: regressionwindow-wanted

Comment 19

10 years ago
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.

Comment 20

10 years ago
bug 406062 was where it was added, although i can't find the bug that made the change to text
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Keywords: regression, regressionwindow-wanted
Resolution: --- → WONTFIX

Comment 21

10 years ago
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.

Comment 22

10 years ago
(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.

Comment 23

10 years ago
(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!!
(Assignee)

Comment 24

10 years ago
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.

Comment 25

10 years ago
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.
(Assignee)

Comment 26

10 years ago
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)

Updated

10 years ago
Assignee: jmathies → nobody
Status: REOPENED → NEW
Component: Widget: Win32 → Editor
QA Contact: win32 → editor
Clearing flags since this got morphed!
Flags: wanted1.9.1+

Comment 28

10 years ago
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.

Comment 30

10 years ago
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.
(Assignee)

Comment 31

10 years ago
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.
(Assignee)

Comment 32

10 years ago
Created attachment 348543 [details] [diff] [review]
simple patch

Here's a simple patch that implements that suggestion.
Assignee: nobody → ted.mielczarek
(Assignee)

Updated

10 years ago
Blocks: 406062
(Assignee)

Comment 33

10 years ago
Created attachment 348545 [details] [diff] [review]
simple patch + test

Add a chrome mochitest.
Attachment #348543 - Attachment is obsolete: true
Attachment #348545 - Flags: superreview?(neil)
Attachment #348545 - Flags: review?(neil)

Comment 34

10 years ago
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.
(Assignee)

Comment 35

10 years ago
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 36

10 years ago
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+
(Assignee)

Comment 37

10 years ago
I can change the test to test those cases as well. Makes sense to have all our assumptions on the table.
(Assignee)

Comment 38

10 years ago
Created attachment 348977 [details] [diff] [review]
patch + updated test

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
(Assignee)

Comment 40

10 years ago
Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/b26e01b47e37
Status: NEW → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
Duplicate of this bug: 466363
(Assignee)

Comment 42

10 years ago
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+
(Assignee)

Comment 44

10 years ago
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
Duplicate of this bug: 470304
Duplicate of this bug: 470562
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.
Keywords: fixed1.9.0.6 → verified1.9.0.6
(Assignee)

Updated

10 years ago
Flags: in-testsuite+

Updated

9 years ago
Duplicate of this bug: 435419
You need to log in before you can comment on or make changes to this bug.