Closed
Bug 513648
Opened 15 years ago
Closed 13 years ago
Pasting URLs with line breaks don't work when pasting into the main window
Categories
(Firefox :: Address Bar, defect)
Tracking
()
VERIFIED
FIXED
Firefox 6
People
(Reporter: szo, Assigned: szo)
Details
Attachments
(1 file, 2 obsolete files)
680 bytes,
patch
|
Details | Diff | Splinter Review |
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,
Assignee | ||
Updated•15 years ago
|
Version: unspecified → 3.0 Branch
Assignee | ||
Updated•14 years ago
|
Version: 3.0 Branch → 3.6 Branch
Assignee | ||
Comment 1•14 years ago
|
||
Updated•14 years ago
|
Assignee: nobody → szo
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Updated•14 years ago
|
Attachment #486886 -
Flags: review?(gavin.sharp)
Comment 2•14 years ago
|
||
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).
Attachment #486886 -
Flags: review?(gavin.sharp) → review-
Assignee | ||
Comment 3•14 years ago
|
||
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•14 years ago
|
||
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, "");
Assignee | ||
Comment 5•14 years ago
|
||
Yes, it looks like exactly what is necessary, thank you!
Assignee | ||
Comment 6•14 years ago
|
||
Attachment #486886 -
Attachment is obsolete: true
Updated•14 years ago
|
Attachment #489443 -
Flags: review+
Comment 7•14 years ago
|
||
Landed with a couple of minor tweaks: http://hg.mozilla.org/mozilla-central/rev/359b8b123e82
Updated•14 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b8
Assignee | ||
Comment 8•13 years ago
|
||
in firefox 4, it doesn't work, because .replace doesn't affect the string, just the return value
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Updated•13 years ago
|
Target Milestone: Firefox 4.0b8 → Future
Version: 3.6 Branch → 4.0 Branch
Comment 10•13 years ago
|
||
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•13 years ago
|
||
(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•13 years ago
|
||
(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.
Status: REOPENED → RESOLVED
Closed: 14 years ago → 13 years ago
Resolution: --- → FIXED
Comment 13•13 years ago
|
||
Landed a bustage fix, for Firefox 6: http://hg.mozilla.org/mozilla-central/rev/ae378603fd43
Updated•13 years ago
|
Target Milestone: Future → Firefox 6
Comment 14•13 years ago
|
||
Verified fixed on Mozilla/5.0 (X11; Linux i686; rv:6.0a1) Gecko/20110421 Firefox/6.0a1
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•