8104 2006-03-31 09:33:45 -0800 REGRESSION (NativeTextField): New text fields should not allow pasting newlines 2006-04-04 01:55:43 -0700 1 1 1 Unclassified WebKit Forms 420+ Mac OS X 10.4 VERIFIED FIXED Regression P1 Normal --- 1 mitz adele sdfisher oldest_to_newest 38086 0 mitz 2006-03-31 09:33:45 -0800 New text fields allow you to paste text containing newlines, and then their height changes to accommodate the multi-line text. Old behavior was to paste the text only up to the first newline. 38105 1 adele 2006-03-31 13:00:10 -0800 *** Bug 8107 has been marked as a duplicate of this bug. *** 38116 2 adele 2006-03-31 14:11:02 -0800 ok- i'm working on this now. 38134 3 7429 adele 2006-03-31 17:10:28 -0800 Created attachment 7429 initial patch posting an initial patch... I ran into a problem while testing a simpler version of this patch. If you select all at cnn.com, and then go to paste that content into the text field, we ended up pasting in a newline at the end. I realized that if we did a test rendering of the plain-text conversion first, and then sent the BeforeTextInserted event (which causes the truncation). I don't like doing another test rendering in ReplaceSelectionCommand though. So I'm trying to find a better solution. Feel free to jump in w/ ideas :) 38180 4 darin 2006-04-01 08:50:11 -0800 (In reply to comment #3) > I don't like doing another test rendering in ReplaceSelectionCommand though. > So I'm trying to find a better solution. Feel free to jump in w/ ideas :) Well, this version seems fine to me; we shouldn't worry unduly about the extra text rendering for now. But if you are doing this, you should move more lines inside the if statement. Specifically, these three don't need to be done twice if you aren't in the plain text case: + range->selectNodeContents(holder.get(), ec); + ASSERT(ec == 0); + text = plainText(range.get()); The use of min/max here is a bit confusing because of the special case for -1. You should probably just write it out like this: if (newLinePosition >= 0 && (truncateLength < 0 || truncateLength > newLinePosition)) truncateLength = newLinePosition; I think that's a little clearer. Another possibility would be to write a local function that is a version of min that handles the -1 value as a special case. static int minPosition(int p1, int p2) { if (p1 < 0) return p2; if (p2 < 0) return p1; return min(p1, p2); } truncateLength = minPosition(truncateLength, newLinePosition); 38181 5 darin 2006-04-01 08:53:12 -0800 (In reply to comment #4) > I think that's a little clearer. Another possibility would be to write a local > function that is a version of min that handles the -1 value as a special case. Or you could initialize truncateLength to text.length() to simplify the code a bit. You're stuck with the "-1" for the find result, but not for your own code. You could even cover find with something that returns the string length instead of -1 when not finding anything. 38353 6 mjs 2006-04-02 23:25:42 -0700 These are all text field regressions so they should all be P1. 38420 7 adele 2006-04-03 14:11:13 -0700 With further testing, and talking w/ Justin, I don't think we actually need to change how ReplaceSelectionCommand works for this fix. Justin reviewed a simpler patch that just does the truncation in HTMLInputElement. 7429 2006-03-31 17:10:28 -0800 2006-03-31 17:10:28 -0800 initial patch patch.txt text/plain 2820 adele SW5kZXg6IGh0bWwvSFRNTElucHV0RWxlbWVudC5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gaHRtbC9IVE1M SW5wdXRFbGVtZW50LmNwcAkocmV2aXNpb24gMTM2MTkpCisrKyBodG1sL0hUTUxJbnB1dEVsZW1l bnQuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC01MCw2ICs1MCw3IEBACiAKIHVzaW5nIG5hbWVzcGFj ZSBFdmVudE5hbWVzOwogdXNpbmcgbmFtZXNwYWNlIEhUTUxOYW1lczsKK3VzaW5nIG5hbWVzcGFj ZSBzdGQ7CiAKIEhUTUxJbnB1dEVsZW1lbnQ6OkhUTUxJbnB1dEVsZW1lbnQoRG9jdW1lbnQgKmRv YywgSFRNTEZvcm1FbGVtZW50ICpmKQogICAgIDogSFRNTEdlbmVyaWNGb3JtRWxlbWVudChpbnB1 dFRhZywgZG9jLCBmKQpAQCAtMTE4MywxMiArMTE4NCwyMSBAQAogICAgICAgICB1bnNpZ25lZCBj dXJyZW50TGVuZ3RoID0gdmFsdWUoKS5sZW5ndGgoKTsKICAgICAgICAgU3RyaW5nIHRleHQgPSBz dGF0aWNfY2FzdDxCZWZvcmVUZXh0SW5zZXJ0ZWRFdmVudCAqPihldnQpLT50ZXh0KCk7CiAgICAg ICAgIGludCBzZWxlY3Rpb25MZW5ndGggPSBkb2N1bWVudCgpLT5mcmFtZSgpLT5zZWxlY3Rpb24o KS50b1N0cmluZygpLmxlbmd0aCgpOwotICAgICAgICAKKyAgICAgICAgaW50IHRydW5jYXRlTGVu Z3RoID0gLTE7CiAgICAgICAgIC8vIFRydW5jYXRlIHRoZSBpbnNlcnRlZCB0ZXh0IGlmIG5lY2Vz c2FyeQogICAgICAgICBpZiAoY3VycmVudExlbmd0aCArIHRleHQubGVuZ3RoKCkgLSBzZWxlY3Rp b25MZW5ndGggPiBtbCkgewogICAgICAgICAgICAgQVNTRVJUKGN1cnJlbnRMZW5ndGggPD0gbWwp OwotICAgICAgICAgICAgdGV4dC50cnVuY2F0ZShtbCAtIGN1cnJlbnRMZW5ndGgpOwotICAgICAg ICB9ICAgICAgICAKKyAgICAgICAgICAgIHRydW5jYXRlTGVuZ3RoID0gbWwgLSBjdXJyZW50TGVu Z3RoOworICAgICAgICB9CisgICAgICAgIAorICAgICAgICAvLyBUcnVuY2F0ZSB0aGUgaW5zZXJ0 ZWQgdGV4dCBhdCB0aGUgZmlyc3QgbmV3IGxpbmUuCisgICAgICAgIGludCBuZXdMaW5lUG9zaXRp b24gPSB0ZXh0LmZpbmQoIlxuIik7CisgICAgICAgIHRydW5jYXRlTGVuZ3RoID0gKHRydW5jYXRl TGVuZ3RoID49IDAgJiYgbmV3TGluZVBvc2l0aW9uID49MCkgPyBtaW4odHJ1bmNhdGVMZW5ndGgs IG5ld0xpbmVQb3NpdGlvbikgOiBtYXgodHJ1bmNhdGVMZW5ndGgsIG5ld0xpbmVQb3NpdGlvbik7 CisgICAgICAgIG5ld0xpbmVQb3NpdGlvbiA9IHRleHQuZmluZCgiXHIiKTsKKyAgICAgICAgdHJ1 bmNhdGVMZW5ndGggPSAodHJ1bmNhdGVMZW5ndGggPj0gMCAmJiBuZXdMaW5lUG9zaXRpb24gPj0w KSA/IG1pbih0cnVuY2F0ZUxlbmd0aCwgbmV3TGluZVBvc2l0aW9uKSA6IG1heCh0cnVuY2F0ZUxl bmd0aCwgbmV3TGluZVBvc2l0aW9uKTsKKyAgICAgICAgCisgICAgICAgIGlmICh0cnVuY2F0ZUxl bmd0aCA+PSAwKQorICAgICAgICAgICAgdGV4dC50cnVuY2F0ZSh0cnVuY2F0ZUxlbmd0aCk7CiAg ICAgfQogICAgIAogICAgIGlmIChtX3R5cGUgPT0gVEVYVCAmJiAoZXZ0LT5pc01vdXNlRXZlbnQo KSB8fCBldnQtPmlzRHJhZ0V2ZW50KCkgfHwgZXZ0LT5pc1doZWVsRXZlbnQoKSkgJiYgcmVuZGVy ZXIoKSkKSW5kZXg6IGVkaXRpbmcvUmVwbGFjZVNlbGVjdGlvbkNvbW1hbmQuY3BwCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIGVkaXRpbmcvUmVwbGFjZVNlbGVjdGlvbkNvbW1hbmQuY3BwCShyZXZpc2lvbiAxMzYx OSkKKysrIGVkaXRpbmcvUmVwbGFjZVNlbGVjdGlvbkNvbW1hbmQuY3BwCSh3b3JraW5nIGNvcHkp CkBAIC0xMzAsMTEgKzEzMCwyMSBAQAogICAgIHJhbmdlLT5zZWxlY3ROb2RlQ29udGVudHMoaG9s ZGVyLmdldCgpLCBlYyk7CiAgICAgQVNTRVJUKGVjID09IDApOwogICAgIFN0cmluZyB0ZXh0ID0g cGxhaW5UZXh0KHJhbmdlLmdldCgpKTsKKworICAgIGlmICghZWRpdGFibGVSb290LT5pc0NvbnRl bnRSaWNobHlFZGl0YWJsZSgpKSB7CisgICAgICAgIHJlbW92ZU5vZGUoaG9sZGVyKTsKKyAgICAg ICAgbV9mcmFnbWVudCA9IGNyZWF0ZUZyYWdtZW50RnJvbVRleHQoZG9jdW1lbnQsIHRleHQuZGVw cmVjYXRlZFN0cmluZygpKTsKKyAgICAgICAgaG9sZGVyID0gaW5zZXJ0RnJhZ21lbnRGb3JUZXN0 UmVuZGVyaW5nKCk7CisgICAgfQorICAgIAorICAgIHJhbmdlLT5zZWxlY3ROb2RlQ29udGVudHMo aG9sZGVyLmdldCgpLCBlYyk7CisgICAgQVNTRVJUKGVjID09IDApOworICAgIHRleHQgPSBwbGFp blRleHQocmFuZ2UuZ2V0KCkpOwogICAgIFN0cmluZyBuZXdUZXh0ID0gdGV4dC5jb3B5KCk7CiAg ICAgLy8gR2l2ZSB0aGUgcm9vdCBhIGNoYW5jZSB0byBjaGFuZ2UgdGhlIHRleHQuCiAgICAgUmVm UHRyPEV2ZW50PiBldnQgPSBuZXcgQmVmb3JlVGV4dEluc2VydGVkRXZlbnQobmV3VGV4dCk7CiAg ICAgZWRpdGFibGVSb290LT5kaXNwYXRjaEV2ZW50KGV2dCwgZWMsIHRydWUpOwotICAgIGlmICh0 ZXh0ICE9IG5ld1RleHQgfHwgIWVkaXRhYmxlUm9vdC0+aXNDb250ZW50UmljaGx5RWRpdGFibGUo KSkgeworICAgIGlmICh0ZXh0ICE9IG5ld1RleHQpIHsKICAgICAgICAgcmVtb3ZlTm9kZShob2xk ZXIpOwogICAgICAgICBtX2ZyYWdtZW50ID0gY3JlYXRlRnJhZ21lbnRGcm9tVGV4dChkb2N1bWVu dCwgbmV3VGV4dC5kZXByZWNhdGVkU3RyaW5nKCkpOwogICAgICAgICBob2xkZXIgPSBpbnNlcnRG cmFnbWVudEZvclRlc3RSZW5kZXJpbmcoKTsK