Last Comment Bug 513648 - Pasting URLs with line breaks don't work when pasting into the main window
: Pasting URLs with line breaks don't work when pasting into the main window
Status: VERIFIED FIXED
:
Product: Firefox
Classification: Client Software
Component: Location Bar (show other bugs)
: 4.0 Branch
: x86 Linux
: -- normal (vote)
: Firefox 6
Assigned To: Szókovács Róbert
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-08-31 02:07 PDT by Szókovács Róbert
Modified: 2011-04-22 04:12 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
proposed fix for the bug (316 bytes, patch)
2010-10-29 04:15 PDT, Szókovács Róbert
gavin.sharp: review-
Details | Diff | Splinter Review
hg diff updated patch (617 bytes, patch)
2010-11-10 00:55 PST, Szókovács Róbert
gavin.sharp: review+
Details | Diff | Splinter Review
hg diff updated patch, for firefox 4 (680 bytes, patch)
2011-04-05 06:18 PDT, Szókovács Róbert
no flags Details | Diff | Splinter Review

Description Szókovács Róbert 2009-08-31 02:07:01 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.13) Gecko/2009080315 Ubuntu/9.04 (jaunty) Firefox/3.0.13
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.13) Gecko/2009080315 Ubuntu/9.04 (jaunty) Firefox/3.0.13

When I copy-paste a multi-line URL from an other window like this:
"
          http://www.fn.hu/csucsfogyaszto/20090829/csucsfogyaszto_turelemjateko
          s_ajtozar/
", the line-break should be removed automatically. This in deed happens, when I paste into the URL bar (good), but don't happen when I paste with middle mouse button into the main window.

Reproducible: Always

Steps to Reproduce:
1. copy URL with linebreak
2. paste with middle mouse click
3. the linebreak is in the url
Actual Results:  
The linebreak is in the URL, opening the page fails.

Expected Results:  
The linebreaks (white spaces) should be removed,
Comment 1 Szókovács Róbert 2010-10-29 04:15:41 PDT
Created attachment 486886 [details] [diff] [review]
proposed fix for the bug
Comment 2 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-10-30 00:35:31 PDT
Comment on attachment 486886 [details] [diff] [review]
proposed fix for the bug

Thanks for the patch!

I think a better option would be to just call trim() on the string returned from readFromClipboard(), if any, before passing it to getShortcutOrURI. That will take care of any leading/trailing whitespace, and match the behavior of the URL bar.

Just for future reference, HG diffs are generally recommended (with settings set from https://developer.mozilla.org/en/Installing_Mercurial#Configuration to provide appropriate diff context and such).
Comment 3 Szókovács Róbert 2010-11-02 02:28:00 PDT
Hi,

trim() would not work, because the problem isn't the leading or trailing whitespaces but the ones in the middle of the string that should not be there, the ones caused by wordwrap made by irc or mail clients, terminal emulators running them, etc.
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-11-02 11:53:36 PDT
Ah, I was confused because "stripsurroundingwhitespace" is the name for the behavior we use for the URL bar, and it sounds like that just does trim(). But it does indeed trim embedded newlines and spaces surrounding them:

http://hg.mozilla.org/mozilla-central/annotate/2a4571a9ef2a/editor/libeditor/text/nsTextEditRules.cpp#l605

So let's just use .replace(/\s*\n\s*/g, "");
Comment 5 Szókovács Róbert 2010-11-03 05:29:22 PDT
Yes, it looks like exactly what is necessary, thank you!
Comment 6 Szókovács Róbert 2010-11-10 00:55:32 PST
Created attachment 489443 [details] [diff] [review]
hg diff updated patch
Comment 7 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-11-11 18:03:17 PST
Landed with a couple of minor tweaks:
http://hg.mozilla.org/mozilla-central/rev/359b8b123e82
Comment 8 Szókovács Róbert 2011-04-05 06:17:05 PDT
in firefox 4, it doesn't work, because .replace doesn't affect the string, just the return value
Comment 9 Szókovács Róbert 2011-04-05 06:18:47 PDT
Created attachment 524054 [details] [diff] [review]
hg diff updated patch, for firefox 4

Updated fix
Comment 10 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-04-16 17:30:41 PDT
Urg, that's embarrassing. replace()'s behavior hasn't changed, JS strings are immutable (and always have been). The patch I landed is just broken :(
Comment 11 Stefan 2011-04-19 13:27:23 PDT
(In reply to comment #4)
> So let's just use .replace(/\s*\n\s*/g, "");

IIRC \s matches \r and \n. So the first "\s*" shall suffice, shouldn't it?
Comment 12 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-04-19 15:48:31 PDT
(In reply to comment #11)
> > So let's just use .replace(/\s*\n\s*/g, "");
> 
> IIRC \s matches \r and \n. So the first "\s*" shall suffice, shouldn't it?

No, because we don't want to remove all whitespace - we only want to remove newlines, and their surrounding whitespace.
Comment 13 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-04-19 15:48:45 PDT
Landed a bustage fix, for Firefox 6:
http://hg.mozilla.org/mozilla-central/rev/ae378603fd43
Comment 14 Vlad [QA] 2011-04-22 04:12:33 PDT
Verified fixed on Mozilla/5.0 (X11; Linux i686; rv:6.0a1) Gecko/20110421 Firefox/6.0a1

Note You need to log in before you can comment on or make changes to this bug.