After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 760497 - Pasting text should remove newlines at front/end of text
Pasting text should remove newlines at front/end of text
Status: RESOLVED FIXED
Product: polari
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Polari maintainers
Polari maintainers
Depends on:
Blocks:
 
 
Reported: 2016-01-11 23:25 UTC by Olav Vitters
Modified: 2016-02-09 19:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
pasteManager: trimmed the text that is to be pasted (1.46 KB, patch)
2016-02-01 23:33 UTC, Rares Visalom
none Details | Review
entryArea: message is trimmed just before it is pasted (1.84 KB, patch)
2016-02-02 15:39 UTC, Rares Visalom
none Details | Review
entryArea: message is trimmed just before it is pasted (1.31 KB, patch)
2016-02-02 17:45 UTC, Rares Visalom
accepted-commit_now Details | Review
entryArea: Trim newline(s)/space(s) in pasted text (1.14 KB, patch)
2016-02-09 18:54 UTC, Rares Visalom
none Details | Review
entryArea: Trim newline(s)/space(s) in pasted text (1.14 KB, patch)
2016-02-09 19:04 UTC, Rares Visalom
none Details | Review
entryArea: Trim newline(s)/space(s) in pasted text (1.13 KB, patch)
2016-02-09 19:10 UTC, Rares Visalom
committed Details | Review

Description Olav Vitters 2016-01-11 23:25:01 UTC
To reproduce:
1. Bug Andre
2. Go to https://www.brussels-city-shuttle.com/en/tickets
3. Copy "Online
from 5 € to max. 14 € per person
(no handling fee)"
4. Try to past this

Actual results:
A. Polari suggests to use public paste server
B. Say paste
C. Open link (https://paste.gnome.org/ppvr11b5e)
D. Copy text again, minus the newlines
E. Paste again
F. Works

Wanted results:
A. Pastes 3 lines of text, no newlines
Comment 1 Rares Visalom 2016-02-01 23:33:39 UTC
Created attachment 320245 [details] [review]
pasteManager: trimmed the text that is to be pasted

Trimming of the text is necessary to prevent unwanted newlines
being sent.

The problem is that when the user selects a multiline text and
wants to paste it in the chatEntry, the pasteManeger takes care of
sending the actual message to gnome pastebin.

The fix is to trim the text just before calling the Utils.gpaste
method from utils.js. A problem that may arise is that the user
still gets prompted with a message length that may be greater than
the one actually being sent. Another possible fix, that also fixes
the length with the issue, would be to trim the text and adjust the
length before emitting the 'test-pasted' signal, in the
vfunc_paste_clipboard function located in entryArea.js.
Comment 2 Kunaal Jain 2016-02-02 13:10:37 UTC
Makes more sense to trim it before calling the pasteManager function.
Comment 3 Florian Müllner 2016-02-02 13:58:31 UTC
(In reply to Kunaal Jain from comment #2)
> Makes more sense to trim it before calling the pasteManager function.

I agree. It's not just that the number of lines we report to the user may be wrong - having that in the signal is just a minor optimization to not compute the value twice, which we could remove again. We may also trigger the paste service unnecessarily - think of the extreme case of pasting five empty lines.
Comment 4 Rares Visalom 2016-02-02 15:39:42 UTC
Created attachment 320269 [details] [review]
entryArea: message is trimmed just before it is pasted

In commit 270cae6, the same bug was fixed, but the trimming was
not done in the right place.

The problem was that the above mentioned commit trimmed the text later
than it should have.

The fix is that the trimming process is done as soon as possible, thus
making it possible for the correct number of pasted lines to be shown.
Note that a message composed of multiple newlines does not get sent to
gnome pastebin, but it is pasted as it is and it gets sent right away, just like a normal written message.
Comment 5 Kunaal Jain 2016-02-02 16:16:34 UTC
Rares the patch you mentioned in commit doesn't exist. Secondly, please rebase your patch on master branch, not on your previous commit :)
Comment 6 Rares Visalom 2016-02-02 17:27:23 UTC
Sorry. I will push another patch soon :D
Comment 7 Rares Visalom 2016-02-02 17:45:23 UTC
Created attachment 320286 [details] [review]
entryArea: message is trimmed just before it is pasted

Trimming of the text is necessary to prevent unwanted newlines
being sent.

The problem is that when the user selects a multiline (greater
than a maximum limit, currently set to 5 by default) text and
wants to paste it in the chatEntry, the pasteManager takes care of
sending the actual message to gnome pastebin.

The fix is to trim the text right after it is retrieved from the
clipboard, thus making sure that no useless newlines will be sent
to gnome pastebin.
Comment 8 Kunaal Jain 2016-02-02 17:52:43 UTC
Looks good to me :) Though I think there is no need of extra line after text.trim().

Florian, Bastian will do the review :)

Commit message should be something like :

"entryArea: Trim extra lines spaces in pasted text

Remove extra lines and space before/after the pasted text to prevent unwanted spaces/lines being sent/pasted."
Comment 9 Florian Müllner 2016-02-02 19:08:19 UTC
Review of attachment 320286 [details] [review]:

(In reply to Kunaal Jain from comment #8)
> Looks good to me :) 

Dto.


> Commit message should be something like :
> 
> "entryArea: Trim extra lines spaces in pasted text
> 
> Remove extra lines and space before/after the pasted text to prevent
> unwanted spaces/lines being sent/pasted."

Yeah. I'm quite fond of detailed commit messages, but I don't think we need it to be that lengthy. The interesting bit is to mention why we trim the text (for anyone who stumbles upon the code and wonders why it's there), not necessarily why it's exactly in that place.
Comment 10 Rares Visalom 2016-02-02 19:14:37 UTC
Should i modify "Trimming of the text is necessary to prevent unwanted newlines being sent." into something else? Or any other part?
Comment 11 Rares Visalom 2016-02-09 18:54:36 UTC
Created attachment 320736 [details] [review]
entryArea: Trim newline(s)/space(s) in pasted text

Remove extra newlines or spaces surrounding the pasted text
to prevent unwanted newlines/spaces from being pasted and
sent to pastebin.

Unnecessary whitespaces around the selected text are not useful
and they usually appear as an odd behaviour when selecting
multiline text from webpages.
Comment 12 Rares Visalom 2016-02-09 19:04:41 UTC
Created attachment 320737 [details] [review]
entryArea: Trim newline(s)/space(s) in pasted text

Remove extra newlines or spaces surrounding the pasted text
to prevent unwanted newlines/spaces from being pasted and
sent to pastebin.

Unnecessary whitespace around the selected text are not useful
and they usually appear as an odd behaviour when selecting
multiline text from webpages.
Comment 13 Rares Visalom 2016-02-09 19:10:09 UTC
Created attachment 320739 [details] [review]
entryArea: Trim newline(s)/space(s) in pasted text

Remove extra newlines or spaces surrounding the pasted text
to prevent unwanted newlines/spaces from being pasted and
sent to pastebin.

Unnecessary whitespace around the selected text is not useful
and it usually appears as an odd behaviour when selecting
multiline text from webpages.
Comment 14 Florian Müllner 2016-02-09 19:57:08 UTC
Attachment 320739 [details] pushed as 165c6fb - entryArea: Trim newline(s)/space(s) in pasted text