Closed
Bug 1320502
Opened 7 years ago
Closed 7 years ago
"Paste and Go" does not work if CRLF exists at the end of clipboard url.
Categories
(Firefox :: Address Bar, defect)
Tracking
()
VERIFIED
FIXED
Firefox 53
Tracking | Status | |
---|---|---|
firefox-esr45 | --- | unaffected |
firefox50 | --- | unaffected |
firefox51 | --- | unaffected |
firefox52 | --- | unaffected |
firefox53 | - | verified |
People
(Reporter: alice0775, Assigned: Gijs)
References
Details
(Keywords: regression)
Attachments
(3 files)
Affected Nightly53.0a1. Not affected Aurora52.0a2. If CRLF exists at the end of clipboard url. "Paste and Go" does not work. It only perform paste. Steps To Reproduce: 1. Copy url to Clipboard from notepad Open attached with notepad Click left side of "h" and hold mouse down and move downward to select a line. Copy to clipboard ( Ctrl+C ) 2. Open New Tab 3. Right Click on Location Bar and Choose "Paste and Go" Actual Results: The url is pasted. The trailing CRLF seems to be ignored and stripped. However, Nothing loads. Expected Results: The trailing CRLF shuld be ignored and stripped. The url should load.
Reporter | ||
Comment 1•7 years ago
|
||
Regressed by: 37a877e840fc Gijs Kruitbosch — Bug 1229426 - avoid dnd of js URIs, r=mak
Blocks: CVE-2017-5458
Flags: needinfo?(gijskruitbosch+bugs)
Reporter | ||
Comment 2•7 years ago
|
||
[Tracking Requested - why for this release]: Broken all function related clipboard of Location Bar. Steps To Reproduce: 1. Copy url to Clipboard: Open attached with notepad.exe Click left side of "h" and hold mouse down and move downward to select a line. Copy to clipboard ( Ctrl+C ) 2. Open New Tab 3. Attempt to open with clipboard url: Right Click on Location Bar and Choose "Paste & Go" Focus Location Bar, paste ( Ctrk+v ) , keypress [Enter] or click Go button[->] Actual Results: The url is pasted. The trailing CRLF seems to be ignored and stripped. However, Nothing loads. Expected Results: The trailing CRLF shuld be ignored and stripped. The url should load.
tracking-firefox53:
--- → ?
Reporter | ||
Comment 3•7 years ago
|
||
STR 1. Copy url to Clipboard: Open attached html Click left side of "h" and hold mouse down and move downward to select a line. Copy to clipboard ( Ctrl+C ) 2. ... 3. ...
Comment 4•7 years ago
|
||
hm, it's the change to stripUnsafeProtocolOnPaste, it now also strips newlines, but that means it will fail the str == fixed_str check we use with it.
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #4) > hm, it's the change to stripUnsafeProtocolOnPaste, it now also strips > newlines, but that means it will fail the str == fixed_str check we use with > it. Ugh, sorry for missing that. I'll check if I can add a test.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(gijskruitbosch+bugs)
Comment hidden (mozreview-request) |
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8814941 [details] Bug 1320502 - fix paste (and go) issues regressed in bug 1229426, https://reviewboard.mozilla.org/r/96000/#review96076 ::: browser/base/content/browser.js:5639 (Diff revision 1) > - return pasteData.replace(/\r?\n/g, "").replace(/^(?:\s*javascript:)+/i, ""); > + let sanitizedPasteData = pasteData.replace(/\r?\n/g, ""); > + let pasteDataNoJS = sanitizedPasteData.replace(/^(?:\s*javascript:)+/i, ""); > + if (pasteDataNoJS.length != sanitizedPasteData.length) { > + return pasteDataNoJS; > + } > + return pasteData; nit: in case you would like to avoid the temp var and length comparison, an alternative could be: let changed = false; let pasteDataNoJS = pasteData.replace(/\r?\n/g, "") .replace(/^(?:\s*javascript:)+/i, () => { changed = true; return ""; }) return changed ? pasteDataNoJS : pasteData; ::: browser/base/content/test/urlbar/browser_pasteAndGo.js:18 (Diff revision 1) > + gURLBar.focus(); > + yield new Promise((resolve, reject) => { > + waitForClipboard(url, function() { > + clipboardHelper.copyString(url); > + }, resolve, > + reject.bind(new Error("Failed to copy string '" + url + "' to clipboard")) does this really work? I've never seen binding a reject to a new Error... Either you meant () => reject(New Error or I learned something new!
Attachment #8814941 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 8•7 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #7) > > + reject.bind(new Error("Failed to copy string '" + url + "' to clipboard")) > > does this really work? I've never seen binding a reject to a new Error... > Either you meant () => reject(New Error > or I learned something new! Re-reading, I think I meant reject.bind(null, new Error(...)). That should work, I think, though the arrow function might be clearer. Why wouldn't it?
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/3a64450cc56a fix paste (and go) issues regressed in bug 1229426, r=mak
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3a64450cc56a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Comment 13•7 years ago
|
||
I have reproduced this bug with Firefox nightly 53.0a1 (2016-11-26)(64-bit) on Windows 10, 64 Bit. The Bug's fix is now verified on latest nightly 53.0a1. Build ID 20161207030204 User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0. [bugday-20161207]
Assignee | ||
Updated•7 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•