196427 2019-03-30 09:42:25 -0700 REGRESSION (r135972): WebCore::TextIterator::plainText() discards value from Document::displayStringModifiedByEncoding() when isDisplayString == true 2023-05-23 08:47:34 -0700 1 1 1 Unclassified WebKit DOM WebKit Nightly Build Unspecified Unspecified NEW https://bugs.webkit.org/show_bug.cgi?id=103295 https://bugs.webkit.org/show_bug.cgi?id=166569 InRadar P2 Normal --- 1 ddkilzer webkit-unassigned ahmad.saleem792 ap cdumez darin rniwa webkit-bug-importer oldest_to_newest 1522778 0 ddkilzer 2019-03-30 09:42:25 -0700 WebCore::TextIterator::plainText() discards value from WebCore::Document::displayStringModifiedByEncoding() when argument isDisplayString == true. <https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/editing/TextIterator.cpp#L2655> String plainText(Position start, Position end, TextIteratorBehavior defaultBehavior, bool isDisplayString) { […] String result = builder.toString(); if (isDisplayString) document->displayStringModifiedByEncoding(result); return result; } Looks like this regressed with this commit: Bug 103295: TextIterator unnecessarily converts 8 bit strings to 16 bits <https://bugs.webkit.org/show_bug.cgi?id=103295> <https://trac.webkit.org/changeset/135972> The method call changed, but the new method returned an updated string instead of modifying it in place: if (isDisplayString && r->ownerDocument()) - r->ownerDocument()->displayBufferModifiedByEncoding(result, bufferLength); + r->ownerDocument()->displayStringModifiedByEncoding(result); And Document::displayBufferModifiedByEncoding() was removed by: Bug 166569: Remove PassRefPtr use from the "dom" directory, related cleanup <https://bugs.webkit.org/show_bug.cgi?id=166569> <https://trac.webkit.org/changeset/210216> 1522779 1 ddkilzer 2019-03-30 09:42:43 -0700 <rdar://problem/49450607> 1522780 2 366364 ddkilzer 2019-03-30 09:44:50 -0700 Created attachment 366364 Patch (no tests; checking EWS) 1522781 3 ddkilzer 2019-03-30 09:49:00 -0700 (In reply to David Kilzer (:ddkilzer) from comment #2) > Created attachment 366364 [details] > Patch (no tests to check EWS) Interesting...the style bot doesn't complain about a missing ChangeLog. 1522789 4 rniwa 2019-03-30 13:25:21 -0700 We need a test for this... 1522838 5 darin 2019-03-31 16:45:36 -0700 I agree. And also, maybe soon we can remove the special yen sign and backslash handling if it’s not needed in practice. 1522844 6 rniwa 2019-03-31 18:34:06 -0700 (In reply to Darin Adler from comment #5) > I agree. > > And also, maybe soon we can remove the special yen sign and backslash > handling if it’s not needed in practice. Unfortunately, I don't think the situation is changing anytime soon about the yen mark :( There is simply too much legacy content out there... 1957299 7 ahmad.saleem792 2023-05-23 05:47:33 -0700 Backslash handling was removed by Blink in this commit: https://src.chromium.org/viewvc/blink?view=revision&revision=159589 1957325 8 ap 2023-05-23 08:47:34 -0700 It's certainly interesting that this bug hasn't been fixed, and we don't seem to be seeing any complaints. That said, I'd be very cautious about putting too much weight Blink decisions in this area. 366364 2019-03-30 09:44:50 -0700 2019-04-18 09:09:07 -0700 Patch (no tests; checking EWS) rdar-49450607-fix-no-test.diff text/plain 1175 ddkilzer ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2RvbS9Eb2N1bWVudC5oIGIvU291cmNlL1dlYkNv cmUvZG9tL0RvY3VtZW50LmgKaW5kZXggZDY3ZjZlODgzODQuLjM4NjJjYjE5ZGJkIDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViQ29yZS9kb20vRG9jdW1lbnQuaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9k b20vRG9jdW1lbnQuaApAQCAtMTEyNSw3ICsxMTI1LDcgQEAgcHVibGljOgogICAgIHZvaWQgc2V0 RGVjb2RlcihSZWZQdHI8VGV4dFJlc291cmNlRGVjb2Rlcj4mJik7CiAgICAgVGV4dFJlc291cmNl RGVjb2RlciogZGVjb2RlcigpIGNvbnN0IHsgcmV0dXJuIG1fZGVjb2Rlci5nZXQoKTsgfQogCi0g ICAgV0VCQ09SRV9FWFBPUlQgU3RyaW5nIGRpc3BsYXlTdHJpbmdNb2RpZmllZEJ5RW5jb2Rpbmco Y29uc3QgU3RyaW5nJikgY29uc3Q7CisgICAgV0VCQ09SRV9FWFBPUlQgU3RyaW5nIGRpc3BsYXlT dHJpbmdNb2RpZmllZEJ5RW5jb2RpbmcoY29uc3QgU3RyaW5nJikgY29uc3QgV0FSTl9VTlVTRURf UkVUVVJOOwogCiAjaWYgRU5BQkxFKERBU0hCT0FSRF9TVVBQT1JUKQogICAgIHZvaWQgc2V0SGFz QW5ub3RhdGVkUmVnaW9ucyhib29sIGYpIHsgbV9oYXNBbm5vdGF0ZWRSZWdpb25zID0gZjsgfQpk aWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvZWRpdGluZy9UZXh0SXRlcmF0b3IuY3BwIGIvU291 cmNlL1dlYkNvcmUvZWRpdGluZy9UZXh0SXRlcmF0b3IuY3BwCmluZGV4IGQ1OGFiYWRjYTNlLi40 NzY0ZGM5MWMwMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvZWRpdGluZy9UZXh0SXRlcmF0 b3IuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2VkaXRpbmcvVGV4dEl0ZXJhdG9yLmNwcApAQCAt MjY3OSw3ICsyNjc5LDcgQEAgU3RyaW5nIHBsYWluVGV4dChQb3NpdGlvbiBzdGFydCwgUG9zaXRp b24gZW5kLCBUZXh0SXRlcmF0b3JCZWhhdmlvciBkZWZhdWx0QmVoYXYKICAgICBTdHJpbmcgcmVz dWx0ID0gYnVpbGRlci50b1N0cmluZygpOwogCiAgICAgaWYgKGlzRGlzcGxheVN0cmluZykKLSAg ICAgICAgZG9jdW1lbnQtPmRpc3BsYXlTdHJpbmdNb2RpZmllZEJ5RW5jb2RpbmcocmVzdWx0KTsK KyAgICAgICAgcmV0dXJuIGRvY3VtZW50LT5kaXNwbGF5U3RyaW5nTW9kaWZpZWRCeUVuY29kaW5n KHJlc3VsdCk7CiAKICAgICByZXR1cm4gcmVzdWx0OwogfQo=