Closed Bug 1402715 Opened 2 years ago Closed 2 years ago

[regression] Spaces are folded when copying from urlbar

Categories

(Firefox :: Address Bar, defect)

53 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 58
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- wontfix
firefox56 --- wontfix
firefox57 --- verified
firefox58 --- verified

People

(Reporter: yfdyh000, Assigned: mats)

References

Details

(Keywords: dataloss, regression)

Attachments

(1 file)

STR:
1. New file like "D:\abc   123.html".
2. Open the file with Firefox.
3. Copy the url from urlbar.


Actual results:
1. got file:///D:/abc 123.html to clipboard.


Expected results:
1. got file:///D:/abc%20%20%20123.html or file:///D:/abc   123.html (bug 1374850).


Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6e6cb36f0e9bb299a68ed000b8e67fa868247599&tochange=9532ebc3faf001a04f12b97cb9e95224898f22ff


I don't know why it happened.
Has Regression Range: --- → yes
Has STR: --- → yes
Flags: needinfo?(mats)
Summary: Spaces are folded when copying from urlbar → [regression] Spaces are folded when copying from urlbar
Backing out bug 1328025 locally seems to fix it.
Assignee: nobody → mats
Blocks: 1328025
Flags: needinfo?(mats)
OS: Unspecified → All
Hardware: Unspecified → All
Attached patch fix+testSplinter Review
Given that 'this.editor' is a nsIPlaintextEditor, it's rather surprising
that its Selection behaves the way it does.  I think we should probably
change that so that selection.toString() treats the selected contents
as plaintext if it's fully inside a plaintext editing host, or something.

Anyway, let's do this manually for now to minimize the risk.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d049650a1546d0ed392a8be24da407bbea4b1bca
Attachment #8911631 - Flags: review?(mak77)
Comment on attachment 8911631 [details] [diff] [review]
fix+test

Review of attachment 8911631 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/urlbarBindings.xml
@@ +974,5 @@
>            let selection = this.editor.selection;
> +          const flags = Ci.nsIDocumentEncoder.OutputPreformatted |
> +                        Ci.nsIDocumentEncoder.OutputRaw;
> +          var selectedVal = selection.QueryInterface(Ci.nsISelectionPrivate)
> +                              .toStringWithFormat("text/plain", flags, 0);

nit: while touching this code s/var/let/
also, please align on dots even if we go slightly (I think it's 1 or 2 chars here) over 80 chars, we are not so strict about it.
Attachment #8911631 - Flags: review?(mak77) → review+
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/45e386a21c2a
Treat URL bar contents as raw text when copying.  r=mak
Comment on attachment 8911631 [details] [diff] [review]
fix+test

Approval Request Comment
[Feature/Bug causing the regression]: bug 1328025
[User impact if declined]: Copying an URL from the location bar to the clipboard will replace consecutive spaces in the result with one space, so the clipboard will contain the wrong URL
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: sure, STR in comment 0
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:no
[Why is the change risky/not risky?]:it's a small change and it seems low risk
[String changes made/needed]:none
Attachment #8911631 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/45e386a21c2a
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Comment on attachment 8911631 [details] [diff] [review]
fix+test

Fix a recent regression and seems low risk, taking it
Should be in 57b4
Attachment #8911631 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
I managed to reproduce the bug using an older version of Nightly (2017-09-24) on WIndows 10x64, Mac OS X 10.11 and Ubuntu 16.04x64.
I retested everything using beta 57.0b8 and latest Nightly 58 on the same platforms and the bug is not reproducing anymore. I think this issue is fixed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.