User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:23.0) Gecko/20100101 Firefox/23.0 (Beta/Release) Build ID: 20130625125232 Steps to reproduce: Since Update to 23. Copy the Text of an JS Alert seems to delete linebreaks in buffer. After Past all Linebreaks are gone. I Need this, so it is important for me. And It works, very well before and in Chrome, Safari, ... System is OSX 10.6.8.

Could you test with a clean profile, please: https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles

I did, and it is the same.

(In reply to Loic from comment #1) > Could you test with a clean profile, please: > https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove- > firefox-profiles I did, and it is the same.

This appears to be a regression in Firefox 23. Steps to Reproduce: 1. Open the Web Console (Ctrl+Shift+K) 2. Type alert("1\n2\n3\n4") and press Enter 3. An alert box appears. Copy the text of the alert and paste it into a text editor Expected Result: 1 2 3 4 Actual Result: 1 2 3 4

Last good nightly: 2013-04-03 First bad nightly: 2013-04-04 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=97cfc16ba5dc&tochange=c232bec6974d

My guess goes to: Drew Willcoxon — Bug 723163 - Improve about:crashes copy & paste by improving XHTML text/plain encoding. r=hsivonen,njn

First BUG I ever announced on firefox. I am really impressed. Your awesome in speed and working prozess! (what do you think would happen if is about IE ;-) )

adw - can you take a look at this FF23 regresssion? We've got a couple of more weeks to take a safe fix, so we'd really appreciate giving this bug priority. Thanks!

Problem seems to be gone in 23b3

(In reply to Christian Menzel from comment #9) > Problem seems to be gone in 23b3 Not in Nightly! Are you able to repro comment #4 in Beta?

(In reply to Christian Menzel from comment #9) > Problem seems to be gone in 23b3 Sorry! mistake: Some linebreaks seem to work, I don't know why: alert(unsescape('Sehr geehrte%28r%29 X X%2C%0A%u2028%u2028Sie sind eine Kundin/ ein Kunde%2C');

yes.

Ok, so the bug isn't fixed.

Hi Christian, Which dialogs did you notice this with? Can you elaborate on your use cases for copying/pasting text out of dialogs? It seems likely that this is a very rare use case, so I wonder whether this is worth fixing.

I am a bit confused of your question. I use this in development prozess of backend software. Because it is sometimes not usefull to implement ajax or lightbox or whatever in a software in development. Sometimes these js-alert are the shortest way to do it. And sometimes it keeps this way longer as thought. But I am confused, cause you say: Let's keep it this way because there is only one guy using this. But I dont't think so. And all Other browsers I tested, don't have this Problem. Do you Know what I mean. I don't want highest Priority or something. But I think this have to be fixed. Everythink worked till 23 so I don't think this could be a extrem hard thing to fix it?

Olli, would you mind reviewing? I'd ask Henri but he's away until 7/29 and we'd like to backport a fix ASAP. Bug 723163 changed nsHTMLCopyEncoder::SetSelection slightly so that it's possible for the caller to tell why the selection forced text/plain. It does this by delaying setting its mime type to text/plain from SetSelection to encoding time when the doc is not an nsIHTMLDocument or not HTML. The alert() box is XUL, so when SelectionCopyHelper checks the encoder's mime type after SetSelection, it's not text/plain, so selForcedTextPlain is false, and SelectionCopyHelper pretty prints the alert()'s selected text. This patch adds another check to SelectionCopyHelper after encoding and sets selForcedTextPlain = true if the doc is not an nsIHTMLDocument.

Comment on attachment 775080 [details] [diff] [review] patch This really needs some tests, so that we don't regress the behavior.

But other than that, this should be ok.

Would be great to get this into the second beta build this week (going to build tomorrow) if you can get an r+ and land to trunk.

I'll try to finish the test(s) and get an r+ before tomorrow but I'm not confident both those things will happen.

This includes one test that verifies copying and pasting in a XUL doc results in verbatim, non-pretty-printed text/plain. There's an existing test_copypaste.html. I tried making a new test based on it but using XHTML. Such a test should pass everything the HTML test does except for text/html encodings, but that turned out not to be the case. There are many divs in that test hidden by display:none or visibility:hidden, and when they're copied and pasted, text/unicode ends up not being on the clipboard because the doc encoder in SelectionCopyHelper produces empty strings. There must be a difference somewhere between how HTML and XHTML are serialized with regard to hidden content, which is something neither this bug nor bug 723163 touches on, and I don't want to track that down right now.

(In reply to Drew Willcoxon :adw from comment #21) > There must be a difference somewhere between how HTML and XHTML > are serialized with regard to hidden content Hmm, that's not quite right. It's true that when content is hidden, the encoder produces empty strings for text/plain for XHTML, but it's also true for HTML. SelectionCopyHelper doesn't put text/unicode on the clipboard in either case, but somehow text/unicode ends up on the clipboard for HTML. It looks like that happens because for HTML, unlike XHTML, SelectionCopyHelper puts text/_moz_htmlcontext and text/_moz_htmlinfo on the clipboard, which causes nsClipboard::HasDataMatchingFlavors to return true for text/unicode, and which nsTransferable::GetTransferData is able to convert to text/unicode.

Here's an alternative patch that's the same as the other one but adds test_copypaste.xhtml, which is just an XHTML version of test_copypaste.html. It tests bug 723163.

How is that alternative then? Looks like it has additional test, but is otherwise the same. I guess I should review the latter patch.

Comment on attachment 778222 [details] [diff] [review] alternative patch with test_copypaste.xhtml also "not an nsIHTMLDocument or not HTML" is odd. Just drop the "or not HTML" and the rest of the comment is a bit odd too. So this patch brings back part of the functionality that got lost in Bug 723163. (there used to be the QI to nsIHTMLDocument) Not happy with code duplication for the test. Could you perhaps check if it was possible to reuse the code, like moving it to a .js file. But r+, I guess.

https://tbpl.mozilla.org/?tree=Try&rev=2da1594d024d (In reply to Olli Pettay [:smaug] from comment #24) > How is that alternative then? Looks like it has additional test, but is > otherwise the same. Like I said. It's an alternative in that there are two and you can choose the one that suits you. Thanks. (In reply to Olli Pettay [:smaug] from comment #25) > "not an nsIHTMLDocument or not HTML" is odd. It accurately describes what the code does: !doc->QI(nsIHTMLDocument) || !doc->IsHTML(). But to be clearer I replaced "not HTML" with "XHTML", which it seems is what !IsHTML() practically means. > Just drop the "or not HTML" and the rest of the comment is a bit odd too. I simplified the rest, so hopefully it's clearer now. > Not happy with code duplication for the test. Could you perhaps check if > it was possible to reuse the code, like moving it to a .js file. Done. > But r+, I guess. Don't let me twist your arm now.

Backed out for B2G failures in test_copypaste.xhtml: https://tbpl.mozilla.org/php/getParsedLog.php?id=25523402&tree=Mozilla-Inbound remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/646cba66eb0c

Is another attempt in the works? We'll want this in our second-to-last Beta that goes to build this Thursday in order to ensure we don't ship this regression that's new to FF23.

Fixed for 23 by bug 897729

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:23.0) Gecko/20100101 Firefox/23.0 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0 Mozilla/5.0 (X11; Linux x86_64; rv:23.0) Gecko/20100101 Firefox/23.0 Verified fixed on 23 beta 9 (buildID: 20130725195523).

THANK YOU.

Oh, the existing test_copypaste.html is disabled on b2g [1], so my new test_copypaste.xhtml should be, too. And I presume the new xul test also. test_copypaste.html is disabled on Android as well, so this disables the new tests there also. I'll land this when I get a chance. https://tbpl.mozilla.org/?tree=Try&rev=526b4ff5611c [1] http://mxr.mozilla.org/mozilla-central/search?string=test_copypaste.html

[Approval Request Comment] Bug caused by (feature/regressing bug #): bug 723163 User impact if declined: described in comment 0; if declined, must back out bug 723163 on beta Testing completed (on m-c, etc.): automated testing on m-c Risk to taking this patch (and alternatives if risky): low String or IDL/UUID changes made by this patch: none Only change from the mozilla-inbound patch is to accommodate small changes in b2g and Android mochitest exclude files: > patching file testing/mochitest/android.json > Hunk #1 succeeded at 8 with fuzz 2 (offset 0 lines). > patching file testing/mochitest/androidx86.json > Hunk #1 FAILED at 6 > 1 out of 1 hunks FAILED -- saving rejects to file testing/mochitest/androidx86.json.rej > patching file testing/mochitest/b2g.json > Hunk #1 FAILED at 154 > 1 out of 1 hunks FAILED -- saving rejects to file testing/mochitest/b2g.json.rej

[Approval Request Comment] Bug caused by (feature/regressing bug #): bug 723163 User impact if declined: described in comment 0 Testing completed (on m-c, etc.): automated testing on m-c Risk to taking this patch (and alternatives if risky): low String or IDL/UUID changes made by this patch: none This is the same as the beta patch but with one small refresh to make it apply to Aurora squeakily cleanly: > patching file testing/mochitest/b2g.json > Hunk #1 succeeded at 158 with fuzz 2 (offset 35 lines).

Can we have a bug to enable those tests on b2g/android, and can we have that bug listed in the manfiests?

https://hg.mozilla.org/releases/mozilla-aurora/rev/f8dceda8a125 https://hg.mozilla.org/releases/mozilla-beta/rev/b3050d05dc96

(In reply to :Ms2ger from comment #38) > Can we have a bug to enable those tests on b2g/android, and can we have that > bug listed in the manfiests? I filed bug 904183, but since the manifests are JSON, and JSON doesn't have comments in the usual sense, I didn't list the bug in the manifests.

Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Firefox/24.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:24.0) Gecko/20100101 Firefox/24.0 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Firefox/24.0 Verified as fixed on Firefox 24 beta 4 (buildID: 20130819170952) and latest Nightly (buildID: 20130820030206).

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 Mozilla/5.0 (X11; Linux i686; rv:25.0) Gecko/20100101 Firefox/25.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:25.0) Gecko/20100101 Firefox/25.0 Verified as fixed with latest Aurora 25.0a2 (Build ID: 20130909004001).

Verified as fixed on the latest Aurora 26.0a2 on Windows 7, Ubuntu 13.04 and Mac OS X 10.8.4: Mozilla/5.0 (Windows NT 6.1; rv:26.0) Gecko/20100101 Firefox/26.0 Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:26.0) Gecko/20100101 Firefox/26.0 Build ID: 20131014004003