5631 2005-11-04 14:36:28 -0800 KWQKHTMLPart::attributedString ignores many tags 2005-12-19 04:25:32 -0800 1 1 1 Unclassified WebKit HTML Editing 420+ Mac OS X 10.4 VERIFIED FIXED P2 Normal --- 1 ap darin evan oldest_to_newest 23746 0 ap 2005-11-04 14:36:28 -0800 Because of what seems to be incorrect if/else nesting, KWQKHTMLPart::attributedString doesn't handle ol, ul, td, th, hr, dd, dl, dt, pre, blockquote, div, p, tr, h1, h2, h3, h4, h5, h6 and img tags. I do not know how to reproduce the problem with shipping WebKit or ToT, but I have tested the fix (for DIV elements) with the proposed implementation of attributedSubstringFromRange (bug 4680) and my input method. 23747 1 4601 ap 2005-11-04 14:36:53 -0800 Created attachment 4601 proposed patch 23749 2 4601 darin 2005-11-04 14:51:22 -0800 Comment on attachment 4601 proposed patch The concept in Tiger was to get out of the business of making attributed strings. AppKit was supposed to handle that for us. The only reason this code is still around is that it was needed on Panther. If we are going to keep and enhance this code that's inside WebCore (which I'm not sure is a good idea), then we need to change it so that the text part of it comes from the text iterator, so the text part of the attributed string will match the plain text in the plain text string. None of that really tells us whether or not to take this particular patch, which might be an OK incremental improvement. 23753 3 evan 2005-11-04 19:38:53 -0800 (In reply to comment #2) Just to muddy the attributed string waters a little more - take a look at what's in KWQAccObject.mm. Yet another way of creating attributed strings. Granted, the names of the attributes are different for AX objects, but same basic idea. So attributed strings are currently created in 3 different ways (and counting?). Maybe time to take another look at the "secret" one that's in AppKit? Do we know *anything* about what it can and can't do? Anyway, playing with this patch, I don't see that it solves the problem with ranges that end with a line break that come from converting an NSRange to a DOMRange. Not sure if that's because of bug 5610 (I think it must be), or something other problem in KWQKHTMLPart::attributedString. 23756 4 ap 2005-11-05 00:38:09 -0800 However, this specific patch only fixes what seems to be a simple typo :) Yes, there are still problems with ranges that end with a newline, but at least newlines at other positions work better with it. 24814 5 4841 ap 2005-11-28 21:24:30 -0800 Created attachment 4841 patch with ignore-whitespace Same patch made with cvs diff -ub for easier reading 24816 6 4601 eric 2005-11-28 21:30:29 -0800 Comment on attachment 4601 proposed patch This patch looks totaly sane to me. Regardless of whether this code is deprecated or not. 26128 7 darin 2005-12-18 14:00:52 -0800 Good point. I didn't look at the patch closely enough when I first reviewed it. Now I get it. Landing it today. 4601 2005-11-04 14:36:53 -0800 2005-11-28 21:30:29 -0800 proposed patch attributedStringTagsPatch.txt text/plain 6327 ap SW5kZXg6IFdlYkNvcmUva3dxL0tXUUtIVE1MUGFydC5tbQo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09ClJDUyBmaWxlOiAv Y3ZzL3Jvb3QvV2ViQ29yZS9rd3EvS1dRS0hUTUxQYXJ0Lm1tLHYKcmV0cmlldmluZyByZXZpc2lv biAxLjY4NApkaWZmIC1wIC11IC1yMS42ODQgV2ViQ29yZS9rd3EvS1dRS0hUTUxQYXJ0Lm1tCi0t LSBXZWJDb3JlL2t3cS9LV1FLSFRNTFBhcnQubW0JMyBOb3YgMjAwNSAxOToxMjowNiAtMDAwMAkx LjY4NAorKysgV2ViQ29yZS9rd3EvS1dRS0hUTUxQYXJ0Lm1tCTQgTm92IDIwMDUgMjI6Mzg6MjUg LTAwMDAKQEAgLTMxNjgsNjAgKzMxNjgsNTkgQEAgTlNBdHRyaWJ1dGVkU3RyaW5nICpLV1FLSFRN TFBhcnQ6OmF0dHJpYgogICAgICAgICAgICAgICAgICAgICAgICAgW3Jlc3VsdCBhcHBlbmRBdHRy aWJ1dGVkU3RyaW5nOiBwYXJ0aWFsU3RyaW5nXTsgICAgICAgICAgICAgICAgCiAgICAgICAgICAg ICAgICAgICAgICAgICBbcGFydGlhbFN0cmluZyByZWxlYXNlXTsKICAgICAgICAgICAgICAgICAg ICAgfQotICAgICAgICAgICAgICAgICAgICBlbHNlIGlmIChuLT5oYXNUYWdOYW1lKG9sVGFnKSB8 fCBuLT5oYXNUYWdOYW1lKHVsVGFnKSkgewotICAgICAgICAgICAgICAgICAgICAgICAgaWYgKCFo YXNOZXdMaW5lKQotICAgICAgICAgICAgICAgICAgICAgICAgICAgIHRleHQgKz0gIlxuIjsKLSAg ICAgICAgICAgICAgICAgICAgICAgIGhhc05ld0xpbmUgPSB0cnVlOwotICAgICAgICAgICAgICAg ICAgICB9IGVsc2UgaWYgKG4tPmhhc1RhZ05hbWUodGRUYWcpIHx8Ci0gICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgbi0+aGFzVGFnTmFtZSh0aFRhZykgfHwKLSAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICBuLT5oYXNUYWdOYW1lKGhyVGFnKSB8fAotICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgIG4tPmhhc1RhZ05hbWUoZGRUYWcpIHx8Ci0gICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgbi0+aGFzVGFnTmFtZShkbFRhZykgfHwKLSAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICBuLT5oYXNUYWdOYW1lKGR0VGFnKSB8fAotICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgIG4tPmhhc1RhZ05hbWUocHJlVGFnKSB8fAotICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgIG4tPmhhc1RhZ05hbWUoYmxvY2txdW90ZVRhZykgfHwKLSAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICBuLT5oYXNUYWdOYW1lKGRpdlRhZykpIHsKLSAgICAgICAgICAgICAg ICAgICAgICAgIGlmICghaGFzTmV3TGluZSkKLSAgICAgICAgICAgICAgICAgICAgICAgICAgICB0 ZXh0ICs9ICdcbic7Ci0gICAgICAgICAgICAgICAgICAgICAgICBoYXNOZXdMaW5lID0gdHJ1ZTsK LSAgICAgICAgICAgICAgICAgICAgfSBlbHNlIGlmIChuLT5oYXNUYWdOYW1lKHBUYWcpIHx8Ci0g ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgbi0+aGFzVGFnTmFtZSh0clRhZykgfHwKLSAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICBuLT5oYXNUYWdOYW1lKGgxVGFnKSB8fAotICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgIG4tPmhhc1RhZ05hbWUoaDJUYWcpIHx8Ci0gICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgbi0+aGFzVGFnTmFtZShoM1RhZykgfHwKLSAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICBuLT5oYXNUYWdOYW1lKGg0VGFnKSB8fAotICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgIG4tPmhhc1RhZ05hbWUoaDVUYWcpIHx8Ci0gICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgbi0+aGFzVGFnTmFtZShoNlRhZykpIHsKLSAgICAgICAg ICAgICAgICAgICAgICAgIGlmICghaGFzTmV3TGluZSkKKyAgICAgICAgICAgICAgICB9IGVsc2Ug aWYgKG4tPmhhc1RhZ05hbWUob2xUYWcpIHx8IG4tPmhhc1RhZ05hbWUodWxUYWcpKSB7CisgICAg ICAgICAgICAgICAgICAgIGlmICghaGFzTmV3TGluZSkKKyAgICAgICAgICAgICAgICAgICAgICAg IHRleHQgKz0gIlxuIjsKKyAgICAgICAgICAgICAgICAgICAgaGFzTmV3TGluZSA9IHRydWU7Cisg ICAgICAgICAgICAgICAgfSBlbHNlIGlmIChuLT5oYXNUYWdOYW1lKHRkVGFnKSB8fAorICAgICAg ICAgICAgICAgICAgICAgICAgICAgbi0+aGFzVGFnTmFtZSh0aFRhZykgfHwKKyAgICAgICAgICAg ICAgICAgICAgICAgICAgIG4tPmhhc1RhZ05hbWUoaHJUYWcpIHx8CisgICAgICAgICAgICAgICAg ICAgICAgICAgICBuLT5oYXNUYWdOYW1lKGRkVGFnKSB8fAorICAgICAgICAgICAgICAgICAgICAg ICAgICAgbi0+aGFzVGFnTmFtZShkbFRhZykgfHwKKyAgICAgICAgICAgICAgICAgICAgICAgICAg IG4tPmhhc1RhZ05hbWUoZHRUYWcpIHx8CisgICAgICAgICAgICAgICAgICAgICAgICAgICBuLT5o YXNUYWdOYW1lKHByZVRhZykgfHwKKyAgICAgICAgICAgICAgICAgICAgICAgICAgIG4tPmhhc1Rh Z05hbWUoYmxvY2txdW90ZVRhZykgfHwKKyAgICAgICAgICAgICAgICAgICAgICAgICAgIG4tPmhh c1RhZ05hbWUoZGl2VGFnKSkgeworICAgICAgICAgICAgICAgICAgICBpZiAoIWhhc05ld0xpbmUp CisgICAgICAgICAgICAgICAgICAgICAgICB0ZXh0ICs9ICdcbic7CisgICAgICAgICAgICAgICAg ICAgIGhhc05ld0xpbmUgPSB0cnVlOworICAgICAgICAgICAgICAgIH0gZWxzZSBpZiAobi0+aGFz VGFnTmFtZShwVGFnKSB8fAorICAgICAgICAgICAgICAgICAgICAgICAgICAgbi0+aGFzVGFnTmFt ZSh0clRhZykgfHwKKyAgICAgICAgICAgICAgICAgICAgICAgICAgIG4tPmhhc1RhZ05hbWUoaDFU YWcpIHx8CisgICAgICAgICAgICAgICAgICAgICAgICAgICBuLT5oYXNUYWdOYW1lKGgyVGFnKSB8 fAorICAgICAgICAgICAgICAgICAgICAgICAgICAgbi0+aGFzVGFnTmFtZShoM1RhZykgfHwKKyAg ICAgICAgICAgICAgICAgICAgICAgICAgIG4tPmhhc1RhZ05hbWUoaDRUYWcpIHx8CisgICAgICAg ICAgICAgICAgICAgICAgICAgICBuLT5oYXNUYWdOYW1lKGg1VGFnKSB8fAorICAgICAgICAgICAg ICAgICAgICAgICAgICAgbi0+aGFzVGFnTmFtZShoNlRhZykpIHsKKyAgICAgICAgICAgICAgICAg ICAgaWYgKCFoYXNOZXdMaW5lKQorICAgICAgICAgICAgICAgICAgICAgICAgdGV4dCArPSAnXG4n OworICAgICAgICAgICAgICAgICAgICAKKyAgICAgICAgICAgICAgICAgICAgLy8gSW4gY2VydGFp biBjYXNlcywgZW1pdCBhIHBhcmFncmFwaCBicmVhay4KKyAgICAgICAgICAgICAgICAgICAgaW50 IGJvdHRvbU1hcmdpbiA9IHJlbmRlcmVyLT5jb2xsYXBzZWRNYXJnaW5Cb3R0b20oKTsKKyAgICAg ICAgICAgICAgICAgICAgaW50IGZvbnRTaXplID0gc3R5bGUtPmh0bWxGb250KCkuZ2V0Rm9udERl ZigpLmNvbXB1dGVkUGl4ZWxTaXplKCk7CisgICAgICAgICAgICAgICAgICAgIGlmIChib3R0b21N YXJnaW4gKiAyID49IGZvbnRTaXplKSB7CisgICAgICAgICAgICAgICAgICAgICAgICBpZiAoIWhh c1BhcmFncmFwaEJyZWFrKSB7CiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgdGV4dCArPSAn XG4nOwotICAgICAgICAgICAgICAgICAgICAgICAgCi0gICAgICAgICAgICAgICAgICAgICAgICAv LyBJbiBjZXJ0YWluIGNhc2VzLCBlbWl0IGEgcGFyYWdyYXBoIGJyZWFrLgotICAgICAgICAgICAg ICAgICAgICAgICAgaW50IGJvdHRvbU1hcmdpbiA9IHJlbmRlcmVyLT5jb2xsYXBzZWRNYXJnaW5C b3R0b20oKTsKLSAgICAgICAgICAgICAgICAgICAgICAgIGludCBmb250U2l6ZSA9IHN0eWxlLT5o dG1sRm9udCgpLmdldEZvbnREZWYoKS5jb21wdXRlZFBpeGVsU2l6ZSgpOwotICAgICAgICAgICAg ICAgICAgICAgICAgaWYgKGJvdHRvbU1hcmdpbiAqIDIgPj0gZm9udFNpemUpIHsKLSAgICAgICAg ICAgICAgICAgICAgICAgICAgICBpZiAoIWhhc1BhcmFncmFwaEJyZWFrKSB7Ci0gICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgIHRleHQgKz0gJ1xuJzsKLSAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgaGFzUGFyYWdyYXBoQnJlYWsgPSB0cnVlOwotICAgICAgICAgICAgICAgICAg ICAgICAgICAgIH0KKyAgICAgICAgICAgICAgICAgICAgICAgICAgICBoYXNQYXJhZ3JhcGhCcmVh ayA9IHRydWU7CiAgICAgICAgICAgICAgICAgICAgICAgICB9Ci0gICAgICAgICAgICAgICAgICAg ICAgICAKLSAgICAgICAgICAgICAgICAgICAgICAgIGhhc05ld0xpbmUgPSB0cnVlOwogICAgICAg ICAgICAgICAgICAgICB9Ci0gICAgICAgICAgICAgICAgICAgIGVsc2UgaWYgKG4tPmhhc1RhZ05h bWUoaW1nVGFnKSkgewotICAgICAgICAgICAgICAgICAgICAgICAgaWYgKHBlbmRpbmdTdHlsZWRT cGFjZSAhPSBuaWwpIHsKLSAgICAgICAgICAgICAgICAgICAgICAgICAgICBpZiAobGlua1N0YXJ0 TG9jYXRpb24gPT0gW3Jlc3VsdCBsZW5ndGhdKSB7Ci0gICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICsrbGlua1N0YXJ0TG9jYXRpb247Ci0gICAgICAgICAgICAgICAgICAgICAgICAgICAg fQotICAgICAgICAgICAgICAgICAgICAgICAgICAgIFtyZXN1bHQgYXBwZW5kQXR0cmlidXRlZFN0 cmluZzpwZW5kaW5nU3R5bGVkU3BhY2VdOwotICAgICAgICAgICAgICAgICAgICAgICAgICAgIFtw ZW5kaW5nU3R5bGVkU3BhY2UgcmVsZWFzZV07Ci0gICAgICAgICAgICAgICAgICAgICAgICAgICAg cGVuZGluZ1N0eWxlZFNwYWNlID0gbmlsOworICAgICAgICAgICAgICAgICAgICAKKyAgICAgICAg ICAgICAgICAgICAgaGFzTmV3TGluZSA9IHRydWU7CisgICAgICAgICAgICAgICAgfQorICAgICAg ICAgICAgICAgIGVsc2UgaWYgKG4tPmhhc1RhZ05hbWUoaW1nVGFnKSkgeworICAgICAgICAgICAg ICAgICAgICBpZiAocGVuZGluZ1N0eWxlZFNwYWNlICE9IG5pbCkgeworICAgICAgICAgICAgICAg ICAgICAgICAgaWYgKGxpbmtTdGFydExvY2F0aW9uID09IFtyZXN1bHQgbGVuZ3RoXSkgeworICAg ICAgICAgICAgICAgICAgICAgICAgICAgICsrbGlua1N0YXJ0TG9jYXRpb247CiAgICAgICAgICAg ICAgICAgICAgICAgICB9Ci0gICAgICAgICAgICAgICAgICAgICAgICBOU0ZpbGVXcmFwcGVyICpm aWxlV3JhcHBlciA9IGZpbGVXcmFwcGVyRm9yRWxlbWVudChzdGF0aWNfY2FzdDxFbGVtZW50SW1w bCAqPihuKSk7Ci0gICAgICAgICAgICAgICAgICAgICAgICBOU1RleHRBdHRhY2htZW50ICphdHRh Y2htZW50ID0gW1tOU1RleHRBdHRhY2htZW50IGFsbG9jXSBpbml0V2l0aEZpbGVXcmFwcGVyOmZp bGVXcmFwcGVyXTsKLSAgICAgICAgICAgICAgICAgICAgICAgIE5TQXR0cmlidXRlZFN0cmluZyAq aVN0cmluZyA9IFtOU0F0dHJpYnV0ZWRTdHJpbmcgYXR0cmlidXRlZFN0cmluZ1dpdGhBdHRhY2ht ZW50OmF0dGFjaG1lbnRdOwotICAgICAgICAgICAgICAgICAgICAgICAgW3Jlc3VsdCBhcHBlbmRB dHRyaWJ1dGVkU3RyaW5nOiBpU3RyaW5nXTsKLSAgICAgICAgICAgICAgICAgICAgICAgIFthdHRh Y2htZW50IHJlbGVhc2VdOworICAgICAgICAgICAgICAgICAgICAgICAgW3Jlc3VsdCBhcHBlbmRB dHRyaWJ1dGVkU3RyaW5nOnBlbmRpbmdTdHlsZWRTcGFjZV07CisgICAgICAgICAgICAgICAgICAg ICAgICBbcGVuZGluZ1N0eWxlZFNwYWNlIHJlbGVhc2VdOworICAgICAgICAgICAgICAgICAgICAg ICAgcGVuZGluZ1N0eWxlZFNwYWNlID0gbmlsOwogICAgICAgICAgICAgICAgICAgICB9CisgICAg ICAgICAgICAgICAgICAgIE5TRmlsZVdyYXBwZXIgKmZpbGVXcmFwcGVyID0gZmlsZVdyYXBwZXJG b3JFbGVtZW50KHN0YXRpY19jYXN0PEVsZW1lbnRJbXBsICo+KG4pKTsKKyAgICAgICAgICAgICAg ICAgICAgTlNUZXh0QXR0YWNobWVudCAqYXR0YWNobWVudCA9IFtbTlNUZXh0QXR0YWNobWVudCBh bGxvY10gaW5pdFdpdGhGaWxlV3JhcHBlcjpmaWxlV3JhcHBlcl07CisgICAgICAgICAgICAgICAg ICAgIE5TQXR0cmlidXRlZFN0cmluZyAqaVN0cmluZyA9IFtOU0F0dHJpYnV0ZWRTdHJpbmcgYXR0 cmlidXRlZFN0cmluZ1dpdGhBdHRhY2htZW50OmF0dGFjaG1lbnRdOworICAgICAgICAgICAgICAg ICAgICBbcmVzdWx0IGFwcGVuZEF0dHJpYnV0ZWRTdHJpbmc6IGlTdHJpbmddOworICAgICAgICAg ICAgICAgICAgICBbYXR0YWNobWVudCByZWxlYXNlXTsKICAgICAgICAgICAgICAgICB9CiAKICAg ICAgICAgICAgICAgICBOU0F0dHJpYnV0ZWRTdHJpbmcgKnBhcnRpYWxTdHJpbmcgPSBbW05TQXR0 cmlidXRlZFN0cmluZyBhbGxvY10gaW5pdFdpdGhTdHJpbmc6dGV4dC5nZXROU1N0cmluZygpXTsK 4841 2005-11-28 21:24:30 -0800 2005-11-28 21:24:30 -0800 patch with ignore-whitespace attributedStringTagsPatchNoWhitespace.txt text/plain 1131 ap SW5kZXg6IEtXUUtIVE1MUGFydC5tbQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09ClJDUyBmaWxlOiAvY3ZzL3Jvb3QvV2Vi Q29yZS9rd3EvS1dRS0hUTUxQYXJ0Lm1tLHYKcmV0cmlldmluZyByZXZpc2lvbiAxLjY5MApkaWZm IC11IC1iIC1yMS42OTAgS1dRS0hUTUxQYXJ0Lm1tCi0tLSBLV1FLSFRNTFBhcnQubW0JMjcgTm92 IDIwMDUgMDc6NTQ6NTIgLTAwMDAJMS42OTAKKysrIEtXUUtIVE1MUGFydC5tbQkyOSBOb3YgMjAw NSAwNToyOTo1NiAtMDAwMApAQCAtMzE2NCw3ICszMTY0LDcgQEAKICAgICAgICAgICAgICAgICAg ICAgICAgIFtyZXN1bHQgYXBwZW5kQXR0cmlidXRlZFN0cmluZzogcGFydGlhbFN0cmluZ107ICAg ICAgICAgICAgICAgIAogICAgICAgICAgICAgICAgICAgICAgICAgW3BhcnRpYWxTdHJpbmcgcmVs ZWFzZV07CiAgICAgICAgICAgICAgICAgICAgIH0KLSAgICAgICAgICAgICAgICAgICAgZWxzZSBp ZiAobi0+aGFzVGFnTmFtZShvbFRhZykgfHwgbi0+aGFzVGFnTmFtZSh1bFRhZykpIHsKKyAgICAg ICAgICAgICAgICB9IGVsc2UgaWYgKG4tPmhhc1RhZ05hbWUob2xUYWcpIHx8IG4tPmhhc1RhZ05h bWUodWxUYWcpKSB7CiAgICAgICAgICAgICAgICAgICAgICAgICBpZiAoIWhhc05ld0xpbmUpCiAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgdGV4dCArPSAiXG4iOwogICAgICAgICAgICAgICAg ICAgICAgICAgaGFzTmV3TGluZSA9IHRydWU7CkBAIC0zMjE4LDcgKzMyMTgsNiBAQAogICAgICAg ICAgICAgICAgICAgICAgICAgW3Jlc3VsdCBhcHBlbmRBdHRyaWJ1dGVkU3RyaW5nOiBpU3RyaW5n XTsKICAgICAgICAgICAgICAgICAgICAgICAgIFthdHRhY2htZW50IHJlbGVhc2VdOwogICAgICAg ICAgICAgICAgICAgICB9Ci0gICAgICAgICAgICAgICAgfQogCiAgICAgICAgICAgICAgICAgTlNB dHRyaWJ1dGVkU3RyaW5nICpwYXJ0aWFsU3RyaW5nID0gW1tOU0F0dHJpYnV0ZWRTdHJpbmcgYWxs b2NdIGluaXRXaXRoU3RyaW5nOnRleHQuZ2V0TlNTdHJpbmcoKV07CiAgICAgICAgICAgICAgICAg W3Jlc3VsdCBhcHBlbmRBdHRyaWJ1dGVkU3RyaW5nOiBwYXJ0aWFsU3RyaW5nXTsK