55396 2011-02-28 12:18:24 -0800 [Mac] Make "Change back to …" contextual menu item work with new autocorrection. 2011-02-28 15:50:47 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Mac (Intel) OS X 10.6 RESOLVED FIXED InRadar P2 Normal --- 1 jiapu.mail webkit-unassigned commit-queue oldest_to_newest 359230 0 jiapu.mail 2011-02-28 12:18:24 -0800 On the version of Mac OS X that uses autocorrection panel, choose "Change back to … " should notify spell checker service about this reversion. <rdar://problem/8836093> 359257 1 84102 jiapu.mail 2011-02-28 12:53:41 -0800 Created attachment 84102 Proposed patch (v1) 359314 2 84102 darin 2011-02-28 13:52:23 -0800 Comment on attachment 84102 Proposed patch (v1) View in context: https://bugs.webkit.org/attachment.cgi?id=84102&action=review > Source/WebCore/editing/Editor.cpp:2496 > + String replacement = plainText(selection.get()); > + client()->recordAutocorrectionResponse(EditorClient::AutocorrectionReverted, replacedString, replacement); You could write this without the local variable. The local variable does provide a bit of documentation, so I guess it could go either way. > Source/WebCore/rendering/InlineTextBox.cpp:1098 > + case DocumentMarker::Replacement: > + computeRectForReplacementMarker(marker, style, font); > + break; Is there a way to test this? > Source/WebCore/rendering/InlineTextBox.cpp:1100 > default: > ASSERT_NOT_REACHED(); If this default wasn’t here, then the compiler would have warned about the fact that the enum value was not handled, so we might have spotted this earlier. Generally speaking, it’s best to structure things so that switches based on enums do not have a default case. Not sure how easy that would be in this case, without sacrificing the “bad enum value” assertion. 359352 3 jiapu.mail 2011-02-28 14:22:50 -0800 (In reply to comment #2) > (From update of attachment 84102 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=84102&action=review > > > > Source/WebCore/rendering/InlineTextBox.cpp:1098 > > + case DocumentMarker::Replacement: > > + computeRectForReplacementMarker(marker, style, font); > > + break; > > Is there a way to test this? > It doesn't seem very easy to test. On low level, current javascript function tests only the existence of the marker, not any detail about the marker. On higher level, I'm not aware of ways to exam if a particular item is in the contextual menu. 359443 4 84102 commit-queue 2011-02-28 15:50:43 -0800 Comment on attachment 84102 Proposed patch (v1) Clearing flags on attachment: 84102 Committed r79936: <http://trac.webkit.org/changeset/79936> 359444 5 commit-queue 2011-02-28 15:50:47 -0800 All reviewed patches have been landed. Closing bug. 84102 2011-02-28 12:53:41 -0800 2011-02-28 15:50:43 -0800 Proposed patch (v1) bug-55396-20110228125340.patch text/plain 3697 jiapu.mail U3VidmVyc2lvbiBSZXZpc2lvbjogNzk2MzQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCAwYTYzZDMxYzRlMDQ0MDJl MzljYTJkMjJjMjZmZWNiNTU0ZWI0MmZmLi45MDIwZGIwNWFmZDQzM2QwNGZmYjgyM2RkOTAwYmJj ZGM0MjE1M2E4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjMgQEAKKzIwMTEtMDItMjggIEppYSBQ dSAgPGpwdUBhcHBsZS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgW01hY10gTWFrZSAiQ2hhbmdlIGJhY2sgdG8g4oCmIiBjb250ZXh0dWFsIG1l bnUgaXRlbSB3b3JrIHdpdGggbmV3IGF1dG9jb3JyZWN0aW9uLgorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTUzOTYKKyAgICAgICAgPHJkYXI6Ly9wcm9i bGVtLzg4MzYwOTM+CisKKyAgICAgICAgVGhlIGNoYW5nZSBpbiBJbmxpbmVUZXh0Qm94LmNwcCBm aXhlcyBhIGJ1Zywgd2hlcmUgdGhlIHJlY3RhbmdsZSBvZiBSZXBsYWNlbWVudCBtYXJrZXIgaXNu J3QKKyAgICAgICAgY2FsY3VsYXRlZC4gV2UgbmVlZCB0aGlzIHRvIGRvIGhpdCB0ZXN0IHdoZW4g ZGVjaWRpbmcgd2hldGhlciB0byBzaG93ICJDaGFuZ2UgYmFjayB0byAuLi4iIG9uCisgICAgICAg IGNvbnRleHR1YWwgbWVudS4KKworICAgICAgICBUaGUgY2hhbmdlIGluIEVkaXRvci5jcHAgaXMg Zm9yIG5vdGlmeWluZyBzcGVsbGNoZWNrZXIgYWJvdXQgdGhlIHJldmVyc2lvbiB3aGVuZXZlciAi Q2hhbmdlCisgICAgICAgIGJhY2sgdG8gLi4uIiBpcyBjbGlja2VkLgorCisgICAgICAgICogZWRp dGluZy9FZGl0b3IuY3BwOgorICAgICAgICAoV2ViQ29yZTo6RWRpdG9yOjpjaGFuZ2VCYWNrVG9S ZXBsYWNlZFN0cmluZyk6CisgICAgICAgICogcmVuZGVyaW5nL0lubGluZVRleHRCb3guY3BwOgor ICAgICAgICAoV2ViQ29yZTo6SW5saW5lVGV4dEJveDo6cGFpbnREb2N1bWVudE1hcmtlcnMpOgor CiAyMDExLTAyLTI0ICBEYW4gQmVybnN0ZWluICA8bWl0ekBhcHBsZS5jb20+CiAKICAgICAgICAg UmV2aWV3ZWQgYnkgRGF2ZSBIeWF0dC4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2VkaXRp bmcvRWRpdG9yLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2VkaXRpbmcvRWRpdG9yLmNwcAppbmRleCBh YzIwM2YxODZlYjZkNDM4M2Y0ZDA5NjNiMDY2MmMzZTM0YTg3ZTBjLi41MmUyZTk4ZDZiNWVkN2Fi MTlmYzA4MTRlN2Q2ZDAwNjIwMzM3OGIyIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9lZGl0 aW5nL0VkaXRvci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvZWRpdGluZy9FZGl0b3IuY3BwCkBA IC0yNDkxLDEwICsyNDkxLDE3IEBAIHZvaWQgRWRpdG9yOjpjaGFuZ2VCYWNrVG9SZXBsYWNlZFN0 cmluZyhjb25zdCBTdHJpbmcmIHJlcGxhY2VkU3RyaW5nKQogICAgIGlmICghc2hvdWxkSW5zZXJ0 VGV4dChyZXBsYWNlZFN0cmluZywgc2VsZWN0aW9uLmdldCgpLCBFZGl0b3JJbnNlcnRBY3Rpb25Q YXN0ZWQpKQogICAgICAgICByZXR1cm47CiAgICAgCisjaWYgU1VQUE9SVF9BVVRPQ09SUkVDVElP Tl9QQU5FTAorICAgIFN0cmluZyByZXBsYWNlbWVudCA9IHBsYWluVGV4dChzZWxlY3Rpb24uZ2V0 KCkpOworICAgIGNsaWVudCgpLT5yZWNvcmRBdXRvY29ycmVjdGlvblJlc3BvbnNlKEVkaXRvckNs aWVudDo6QXV0b2NvcnJlY3Rpb25SZXZlcnRlZCwgcmVwbGFjZWRTdHJpbmcsIHJlcGxhY2VtZW50 KTsKKyNlbmRpZgogICAgIFRleHRDaGVja2luZ1BhcmFncmFwaCBwYXJhZ3JhcGgoc2VsZWN0aW9u KTsKICAgICByZXBsYWNlU2VsZWN0aW9uV2l0aFRleHQocmVwbGFjZWRTdHJpbmcsIGZhbHNlLCBm YWxzZSk7CiAgICAgUmVmUHRyPFJhbmdlPiBjaGFuZ2VkUmFuZ2UgPSBwYXJhZ3JhcGguc3VicmFu Z2UocGFyYWdyYXBoLmNoZWNraW5nU3RhcnQoKSwgcmVwbGFjZWRTdHJpbmcubGVuZ3RoKCkpOwog ICAgIGNoYW5nZWRSYW5nZS0+c3RhcnRDb250YWluZXIoKS0+ZG9jdW1lbnQoKS0+bWFya2Vycygp LT5hZGRNYXJrZXIoY2hhbmdlZFJhbmdlLmdldCgpLCBEb2N1bWVudE1hcmtlcjo6UmVwbGFjZW1l bnQsIFN0cmluZygpKTsKKyNpZiBTVVBQT1JUX0FVVE9DT1JSRUNUSU9OX1BBTkVMCisgICAgY2hh bmdlZFJhbmdlLT5zdGFydENvbnRhaW5lcigpLT5kb2N1bWVudCgpLT5tYXJrZXJzKCktPmFkZE1h cmtlcihjaGFuZ2VkUmFuZ2UuZ2V0KCksIERvY3VtZW50TWFya2VyOjpTcGVsbENoZWNraW5nRXhl bXB0aW9uKTsKKyNlbmRpZgogfQogCiAjZW5kaWYKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3Jl L3JlbmRlcmluZy9JbmxpbmVUZXh0Qm94LmNwcCBiL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9J bmxpbmVUZXh0Qm94LmNwcAppbmRleCBmYjVmMmJlY2MwNDY3MzM0ZjAyNDdiMTBjN2YyNzJkNDBh OTJmMWU3Li4zYzNlNDUwNTM3OWNkZGNlYjdmOTVhMTY4ZGEwYWFhZjQ3OWRiNTliIDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvSW5saW5lVGV4dEJveC5jcHAKKysrIGIvU291 cmNlL1dlYkNvcmUvcmVuZGVyaW5nL0lubGluZVRleHRCb3guY3BwCkBAIC0xMDU4LDYgKzEwNTgs NyBAQCB2b2lkIElubGluZVRleHRCb3g6OnBhaW50RG9jdW1lbnRNYXJrZXJzKEdyYXBoaWNzQ29u dGV4dCogcHQsIGNvbnN0IEZsb2F0UG9pbnQmCiAgICAgICAgICAgICBjYXNlIERvY3VtZW50TWFy a2VyOjpHcmFtbWFyOgogICAgICAgICAgICAgY2FzZSBEb2N1bWVudE1hcmtlcjo6U3BlbGxpbmc6 CiAgICAgICAgICAgICBjYXNlIERvY3VtZW50TWFya2VyOjpDb3JyZWN0aW9uSW5kaWNhdG9yOgor ICAgICAgICAgICAgY2FzZSBEb2N1bWVudE1hcmtlcjo6UmVwbGFjZW1lbnQ6CiAgICAgICAgICAg ICAgICAgaWYgKGJhY2tncm91bmQpCiAgICAgICAgICAgICAgICAgICAgIGNvbnRpbnVlOwogICAg ICAgICAgICAgICAgIGJyZWFrOwpAQCAtMTA5MCw5ICsxMDkxLDExIEBAIHZvaWQgSW5saW5lVGV4 dEJveDo6cGFpbnREb2N1bWVudE1hcmtlcnMoR3JhcGhpY3NDb250ZXh0KiBwdCwgY29uc3QgRmxv YXRQb2ludCYKICAgICAgICAgICAgICAgICBwYWludFRleHRNYXRjaE1hcmtlcihwdCwgYm94T3Jp Z2luLCBtYXJrZXIsIHN0eWxlLCBmb250KTsKICAgICAgICAgICAgICAgICBicmVhazsKICAgICAg ICAgICAgIGNhc2UgRG9jdW1lbnRNYXJrZXI6OkNvcnJlY3Rpb25JbmRpY2F0b3I6Ci0gICAgICAg ICAgICAgICAgY29tcHV0ZVJlY3RGb3JSZXBsYWNlbWVudE1hcmtlcihtYXJrZXIsIHN0eWxlLCBm b250KTsKICAgICAgICAgICAgICAgICBwYWludFNwZWxsaW5nT3JHcmFtbWFyTWFya2VyKHB0LCBi b3hPcmlnaW4sIG1hcmtlciwgc3R5bGUsIGZvbnQsIGZhbHNlKTsKICAgICAgICAgICAgICAgICBi cmVhazsKKyAgICAgICAgICAgIGNhc2UgRG9jdW1lbnRNYXJrZXI6OlJlcGxhY2VtZW50OgorICAg ICAgICAgICAgICAgIGNvbXB1dGVSZWN0Rm9yUmVwbGFjZW1lbnRNYXJrZXIobWFya2VyLCBzdHls ZSwgZm9udCk7CisgICAgICAgICAgICAgICAgYnJlYWs7CiAgICAgICAgICAgICBkZWZhdWx0Ogog ICAgICAgICAgICAgICAgIEFTU0VSVF9OT1RfUkVBQ0hFRCgpOwogICAgICAgICB9Cg==