Bug 125440 - textimport: copy & paste one row is different from multiple rows
Summary: textimport: copy & paste one row is different from multiple rows
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.3.0.4 release
Hardware: All All
: medium normal
Assignee: Samuel Mehrbrodt (allotropia)
URL:
Whiteboard: target:7.0.0 target:7.2.0
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2019-05-22 05:57 UTC by Oliver Brinzing
Modified: 2022-09-29 23:03 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:


Attachments
multiple_rows_textimport (61.28 KB, image/png)
2019-05-22 05:58 UTC, Oliver Brinzing
Details
one_row_paste (25.93 KB, image/png)
2019-05-22 05:58 UTC, Oliver Brinzing
Details
screenshot from viewfun5.cxx (165.30 KB, image/png)
2019-05-24 17:40 UTC, Oliver Brinzing
Details
screenshot from impex.cxx (96.64 KB, image/png)
2019-05-24 17:41 UTC, Oliver Brinzing
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Brinzing 2019-05-22 05:57:25 UTC
steps to reproduce:

- open new spreadsheet
- copy rows to clipboard:

  "Row1"	""	""	"50.000"
  "Row2"	""	""	"-50.000"

- paste data into spreadsheet
-> text Import dialog will open, make selection as shwon in attached screenshots
-> data is pasted as values.

do same with one row:
-> no text Import Dialog appear, datais pasted as text
Comment 1 Oliver Brinzing 2019-05-22 05:58:05 UTC
Created attachment 151584 [details]
multiple_rows_textimport
Comment 2 Oliver Brinzing 2019-05-22 05:58:42 UTC
Created attachment 151585 [details]
one_row_paste
Comment 3 m_a_riosv 2019-05-22 14:05:58 UTC
Repro
Version: 6.2.4.1 (x64)
Build ID: 170a9c04e0ad25cd937fc7a913bb06bf8c75c11d
CPU threads: 4; OS: Windows 10.0; UI render: GL; VCL: win; 
Locale: es-ES (es_ES); UI-Language: en-US Calc: threaded
Versión: 6.3.0.0.alpha1 (x64)
Id. de compilación: 547edd20e527fb02900f6174973770d26306e2e7
Subprocs. CPU: 4; SO: Windows 10.0; Repres. IU: GL; VCL: win; 
Configuración regional: es-ES (es_ES); Idioma de IU: es-ES Calc: threaded
Comment 4 Xisco Faulí 2019-05-23 10:00:24 UTC
Also reproduced in

Version: 5.2.0.0.alpha1+
Build ID: 5b168b3fa568e48e795234dc5fa454bf24c9805e
CPU Threads: 4; OS Version: Linux 4.15; UI Render: default; 
Locale: ca-ES (ca_ES.UTF-8)

in

Version 4.1.0.0.alpha0+ (Build ID: efca6f15609322f62a35619619a6d5fe5c9bd5a)

the 2 empty cells in the middle are respected
Comment 5 raal 2019-05-23 11:39:04 UTC
repro Version: 4.2.0.0.alpha1+
Build ID: fc8f44e82de4ebdd50ac5fbb9207cd1a59a927e3
Comment 6 Oliver Brinzing 2019-05-23 17:20:12 UTC
no repro with Version 3.6.7.2 (Build ID: e183d5b)
(using paste special)
Comment 7 Oliver Brinzing 2019-05-24 17:40:37 UTC
Created attachment 151662 [details]
screenshot from viewfun5.cxx
Comment 8 Oliver Brinzing 2019-05-24 17:41:14 UTC
Created attachment 151663 [details]
screenshot from impex.cxx
Comment 9 Oliver Brinzing 2019-05-24 17:51:00 UTC
started to debug:

viewfun5.cxx, line 341:
- csv text import dialog will only open if there is more than one line pasted
-> this is same code as in AOO 4.1.5

impex.cxx, line 1291
- pExtOptions is not set, maybe the root cause?

noticed: 
paste *special* one line with AOO 4.1.5/LO 3.6.7.2 results in pasting data
splitted into cells without opening text import dialog, while a simple paste
will copy the line in one cell
Comment 10 Oliver Brinzing 2019-05-25 13:10:32 UTC
noticed: 

- only tab separated values will be splitted and pasted into single cells 
- AOO 4.1.5/LO 3.6.7.2 will detect numbers and remove enclosing quotes

AOO 4.1.5/LO 3.6.7.2
- tab
  "Row1"	"1"	"2" -> Row1 | 1 | 2

- other delimeters:
  "Row1";"1";"2" -> "Row1";"1";"2" (AOO 4.1.5)
                 -> Row            (LO 3.6.7.2)

since LO 4
- tab
  "Row1"	"1"	"2" -> "Row1" | "1" | "2"

- other delimeters:
  "Row1";"1";"2" -> "Row1";"1";"2" 

AOO4.1.5 calls a method setString() with parameter bDetectNumberformat=true:
 sal_Bool ScDocument::SetString( SCCOL nCol, SCROW nRow, SCTAB nTab, const 
                                 String& rString, SvNumberFormatter* pFormatter,
                                 bool bDetectNumberFormat )

while LO setString() method misses this parameter:
sc\source\core\data\document.cxx:
 bool ScDocument::SetString( SCCOL nCol, SCROW nRow, SCTAB nTab, const 
                             OUString& rString, const ScSetStringParam* pParam )
Comment 11 Samuel Mehrbrodt (allotropia) 2020-01-22 10:24:45 UTC
Regression from

commit 9dba77d1b5bb2d513e8d6b67c83dc6e858e35f66
Author: Eike Rathke <erack@redhat.com>
Date:   Mon Mar 18 15:53:21 2013 +0100

    resolved #i119960# paste single line with quotes
    
    Change-Id: I8dae849a39a464a0aaef7e775956c04c53038a48

https://cgit.freedesktop.org/libreoffice/core/commit/?id=9dba77d1b5bb2d513e8d6b67c83dc6e858e35f66
Comment 12 Eike Rathke 2020-01-22 13:59:05 UTC
Note also related

commit f435cdba5b4ea98dc30b4b4879de21a542c8a84d
Author:     Eike Rathke <erack@redhat.com>
CommitDate: Mon Nov 13 18:43:35 2017 +0100

    Resolves: tdf#113571 paste-special "Unformatted text [TSV-Calc]", tdf#32213
Comment 13 Eike Rathke 2020-01-22 14:03:31 UTC
Also

commit 329eeefcbd65ea88f0c8c3f034d49ba73045d059
Author:     Eike Rathke <erack@redhat.com>
CommitDate: Tue Nov 14 17:40:15 2017 +0100

    Distinguish single/multiple cell copy for plain text, tdf#113571 follow-up
Comment 14 Eike Rathke 2020-01-22 17:20:02 UTC
Note that for *one* line of plain text
"One"{TAB}""{TAB}"Three"
I'd expect the three cells
"One"|""|"Three"
*including* the double quote characters.

This because the dialog is not invoked for one line (and shouldn't) and to avoid data loss when copying a single line from a text editor.
Of course the current result of
"One"|""{TAB}"Three"
in two cells is wrong.
Comment 15 Samuel Mehrbrodt (allotropia) 2020-01-23 15:38:19 UTC
(In reply to Eike Rathke from comment #14)
> Note that for *one* line of plain text
> "One"{TAB}""{TAB}"Three"
> I'd expect the three cells
> "One"|""|"Three"
> *including* the double quote characters.
> 
> This because the dialog is not invoked for one line (and shouldn't) and to
> avoid data loss when copying a single line from a text editor.
> Of course the current result of
> "One"|""{TAB}"Three"
> in two cells is wrong.

FWIW, LibreOffice up to 4.1, OpenOffice and Excel all import these values without the quotes.

Maybe a question for UX?
Comment 16 Heiko Tietze 2020-01-24 09:31:52 UTC
Would expect "One"{TAB}""{TAB}"Three" to become One||Three without quotes. And "FWIW, LibreOffice up to 4.1, OpenOffice and Excel all import these values without the quotes" is a strong indicator to do so.

Don't get the point of potential data loss.
Comment 17 Eike Rathke 2020-02-04 15:29:53 UTC
Data loss in the sense that if the result is One||Three then the quote characters are lost and there's nothing the user could do to prevent that. It's easier to remove them if wanted than to figure out where to add them if needed.

Maybe a compromise could be to raise the text import dialog if and only if the one-line data contains quoted fields. Not raising the dialog so far on one line of data is on purpose, always raising it would be a usability issue and interfere with workflows.
Comment 18 Samuel Mehrbrodt (allotropia) 2020-02-12 07:10:28 UTC
(In reply to Eike Rathke from comment #17)
> Data loss in the sense that if the result is One||Three then the quote
> characters are lost and there's nothing the user could do to prevent that.
> It's easier to remove them if wanted than to figure out where to add them if
> needed.
> 
> Maybe a compromise could be to raise the text import dialog if and only if
> the one-line data contains quoted fields. Not raising the dialog so far on
> one line of data is on purpose, always raising it would be a usability issue
> and interfere with workflows.

Another idea is to add a paste option which shows the text import dialog (Excel also has that as an option how to paste).

So the default could be as suggested in my patch, but users can still override the default behavior by manually triggering the text import dialog with the paste option.

Would that work for you?
Comment 19 Eike Rathke 2020-02-12 16:01:24 UTC
(In reply to Samuel Mehrbrodt (CIB) from comment #18)
> Another idea is to add a paste option which shows the text import dialog
> (Excel also has that as an option how to paste).
Might also be possible.
Comment 20 Heiko Tietze 2020-02-18 13:39:15 UTC
(In reply to Samuel Mehrbrodt (CIB) from comment #18)
> Another idea is to add a paste option which shows the text import dialog
> (Excel also has that as an option how to paste).

Eike's proposal is quite intuitive. Cannot think of a user who remembers a key combination to trigger the dialog in a certain situation.
Comment 21 Samuel Mehrbrodt (allotropia) 2020-03-03 13:18:47 UTC
(In reply to Heiko Tietze from comment #20)
> (In reply to Samuel Mehrbrodt (CIB) from comment #18)
> > Another idea is to add a paste option which shows the text import dialog
> > (Excel also has that as an option how to paste).
> 
> Eike's proposal is quite intuitive. Cannot think of a user who remembers a
> key combination to trigger the dialog in a certain situation.

Yeah that would be awkward, true. But I was not talking about a random key combination, rather about a paste special option, as we already have a few in the context menu (right click in cell, "Paste special" submenu.

I now have a patch at [1] which adds a new entry there "Use text import dialog". This allows to raise the text import dialog, so users can choose how their input is going to be pasted, if the default does not fit them.

So with my two patches [1] and [2], the default is now again as it is in OpenOffice and in Excel, but the user can still choose to override the default using the "Use text import dialog" option.

[1] https://gerrit.libreoffice.org/c/core/+/89886
[2] https://gerrit.libreoffice.org/c/core/+/87194
Comment 22 Commit Notification 2020-03-11 11:44:09 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/dc4a291b900fd4786e9e9e7515c3a8675a8c3467

tdf#125440 When inserting TSV, consider quotes as field markers

It will be available in 7.0.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 23 Commit Notification 2020-03-11 11:45:30 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/eeeed08b428685dd7934c4576f1bf4aa7436f96a

tdf#125440 Allow raising text import dialog for paste

It will be available in 7.0.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 24 Xisco Faulí 2020-03-19 17:13:52 UTC
Hi Samuel,
Should your fix be backported to libreoffice-6-4 branch or you prefer to wait for LibreOffice 7.0 ?
Comment 25 Commit Notification 2021-05-06 13:45:31 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/ae5e62ee70ca22e3bf89becd01f6067bfaa81921

tdf#125440: sc: Add UItest

It will be available in 7.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.