4681 2005-08-26 23:20:19 -0700 -[WebHTMLView insertText:] needs to support NSTextInputReplacementRangeAttributeName 2007-07-01 22:28:47 -0700 1 1 1 Unclassified WebKit HTML Editing 420+ Mac OS X 10.4 RESOLVED DUPLICATE 13664 P2 Normal --- 1 evan webkit-unassigned ap ian oliver oldest_to_newest 17735 0 evan 2005-08-26 23:20:19 -0700 The current comment in -insertText reads: // we don't yet support inserting an attributed string but input methods // don't appear to require this. This is not true. In order for kEventClassTextInput/kEventTextInputUpdateActiveInputArea events that specify the kEventParamTextInputSendReplaceRange parameter, the "hidden" NSTextInputReplacementRangeAttributeName attribute must be extracted and used. Seems like a simple change to select that range before calling _insertText basically does the trick. There are some issues with space runs (a separate problem), so there may be a better way. But the following is a big improvement: - (void)insertText:(id)string { NSString *text; if ([string isKindOfClass:[NSAttributedString class]]) { text = [string string]; // we need to support the attribute:NSTextInputReplacementRangeAttributeName if it exists. // The AppKit adds a 'secret' property to the string that contains the replacement // range. The replacement range is the range of the text that should be replaced // with the new string. NSString *rangeString = [string attribute:NSTextInputReplacementRangeAttributeName atIndex:0 longestEffectiveRange:NULL inRange:NSMakeRange(0, [text length])]; if (rangeString) { [[self _bridge] selectNSRange:NSRangeFromString(rangeString)]; } } else { text = string; } [self _insertText:text selectInsertedText:NO]; } Again, probably not enough error checking, and instead of calling _insertText some sort of string replacement method should be called instead. But this alone improves things with my input method. Kotoeri also uses this event, if I'm not mistaken. I filed a radar on this one long, long ago... (Note I've edited the comment to read "the" instead of "the the" - this comment was copied and pasted from -setMarkedText:selectedRange). 19447 1 3874 evan 2005-09-12 01:36:19 -0700 Created attachment 3874 proposed patch 20713 2 mjs 2005-09-27 23:38:46 -0700 Sounds like a good patch, but how to test this? What steps with what input method will demonstrate it? 20716 3 3874 mjs 2005-09-28 00:56:53 -0700 Comment on attachment 3874 proposed patch The patch looks good but r- for lack of test case or even steps to reproduce. 20717 4 evan 2005-09-28 01:04:09 -0700 Indeed, not too many input methods use this. Here's how to reproduce the problem and verify the fix: Download Spell Catcher X (my product, the one input method I know that uses this - though I think Kotoeri might as well): <http://www.rainmakerinc.com/downloads/> Install it, run it in trial mode. Test with whatever app you like - I use Blot, use Safari with contenteditable, or even try with Mail on Tiger. The critical setting: In Spell Catcher Preferences, Interactive pane, Typing tab, the "Make replacements directly..." checkbox controls whether UAIA is called with a replacement range (no backspacing) or not (backspaces over the error/shorthand abbreviation). Run with that checkbox on (the default). Activate Spell Catcher's input method. Type a word that will automatically get corrected - like "teh" or "recieve" (US English, anyway) Try with the shipping WebKit. See that there are remnants of the word you type. Try with a WebKit built with this patch. See that it works (no remnants, no backspacing). 20852 5 3874 mjs 2005-09-28 14:08:04 -0700 Comment on attachment 3874 proposed patch Putting back in review? since steps to reproduce are now provided 20977 6 3874 darin 2005-09-30 07:45:34 -0700 Comment on attachment 3874 proposed patch This patch doesn't really match the title of this bug. It's great to support NSTextInputReplacementRangeAttributeName, but is that really "supporting inserting attributed strings"? Another way to ask this is: "Is this the only attribute we need to support here?" Was this bug originally filed specifically just for the replacement range attribute or are there other pending issues? 21044 7 evan 2005-09-30 17:59:03 -0700 You're quite right, the title of the bug more closely matches the comment that was intially in the code - as it say's inserting attributed strings isn't supported because input methods don't require it. So sure, the basics are that the NSTextInputReplacementRangeAttributeName attribute needs to be supported. Other attributes, I honestly have no idea. Not sure if any of the TSM Glyph Access stuff might funnel through here - my input method doesn't use it. I think that the Character Palette may (insert with font), but haven't tested that a whole lot. I may well take a look, though. And YES, there's no way my proposed patch is good enough. It's a proof that there is a way to get UAIA with kEventParamTextInputSendReplaceRange working in WebView. There ARE other issues (as stated in my original comment) related to the way space runs are dealt with. That's a more general -insertText issue - I have a radar on that filed, and suppose I need to report it here. I'll need help on that one, as the whole deal with WebView and spaces/option spaces/space runs is a bit mysterious to me. To correctly fix this one, I need a bit of help as well. PROPERLY implemented, if there's a replacement range, the behavior should be the same as how TextEdit's "Paste and Match Style" command works. I haven't done enough testing to see whether this patch behaves that way already. I suspect it doesn't. The key idea is that replacing a word that's (say) bold should leave you with a bold replacement. Additionally, we have to make sure that we don't behave like NSTextView in one way - it will scroll the selection into view regardless of the replacement range. That's bad, it should be possible to replace any arbitrary range, independent of the current selection or insertion point, without any such side-effects. I'll be taking another look at this in the next day or so, and I'll write up the other issue that insertText has in general with text that contains space runs (easy to see for yourself if you do some testing with Spell Catcher X, or even TypeIt4Me - don't think any of Apple's input methods will do the trick here). 21215 8 3874 darin 2005-10-02 20:53:52 -0700 Comment on attachment 3874 proposed patch OK, lets work on a version of this that gets as many as possible of the fine points Evan mentions working. 5824 9 oliver 2007-06-30 17:32:59 -0700 *** This bug has been marked as a duplicate of 13664 *** 5799 10 ap 2007-06-30 23:52:34 -0700 Although fixing this issue would hide the symptoms of bug 13664, they are not duplicates (see bug 13664 comment 19). 5784 11 oliver 2007-07-01 15:20:15 -0700 No, this is a duplicate. The problems seen in 13364 are very blatantly due to us not correctly handling the case where we've been given a replacement range. That is the bug. 5754 12 oliver 2007-07-01 22:28:14 -0700 Whoops, should be resolved duplicate 5755 13 oliver 2007-07-01 22:28:47 -0700 re-duplicating *** This bug has been marked as a duplicate of 13664 *** 3874 2005-09-12 01:36:19 -0700 2005-10-02 20:53:52 -0700 proposed patch insertText.txt text/plain 1431 evan SW5kZXg6IFdlYkhUTUxWaWV3Lm0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTogL2N2cy9yb290L1dlYktp dC9XZWJWaWV3LnN1YnByb2ovV2ViSFRNTFZpZXcubSx2CnJldHJpZXZpbmcgcmV2aXNpb24gMS40 NjcKZGlmZiAtcCAtdSAtcjEuNDY3IFdlYkhUTUxWaWV3Lm0KLS0tIFdlYkhUTUxWaWV3Lm0JNSBT ZXAgMjAwNSAyMjoyNjowNSAtMDAwMAkxLjQ2NworKysgV2ViSFRNTFZpZXcubQkxMiBTZXAgMjAw NSAwODozODowOSAtMDAwMApAQCAtNTEwOCw4ICs1MTA4LDE3IEBAIHN0YXRpYyBOU0FycmF5ICp2 YWxpZEF0dHJpYnV0ZXMgPSBuaWw7CiAgICAgTlNTdHJpbmcgKnRleHQ7CiAgICAgaWYgKFtzdHJp bmcgaXNLaW5kT2ZDbGFzczpbTlNBdHRyaWJ1dGVkU3RyaW5nIGNsYXNzXV0pIHsKIAl0ZXh0ID0g W3N0cmluZyBzdHJpbmddOwotICAgICAgICAvLyB3ZSBkb24ndCB5ZXQgc3VwcG9ydCBpbnNlcnRp bmcgYW4gYXR0cmlidXRlZCBzdHJpbmcgYnV0IGlucHV0IG1ldGhvZHMKLSAgICAgICAgLy8gZG9u J3QgYXBwZWFyIHRvIHJlcXVpcmUgdGhpcy4KKwkKKyAgICAgICAgLy8gV2UgbmVlZCB0byBzdXBw b3J0IHRoZSBhdHRyaWJ1dGU6TlNUZXh0SW5wdXRSZXBsYWNlbWVudFJhbmdlQXR0cmlidXRlTmFt ZSBpZiBpdCBleGlzdHMuCisgICAgICAgIC8vIFRoaXMgYXR0cmlidXRlIGNvcnJlc3BvbmRzIHRv IHRoZSBrRXZlbnRQYXJhbVRleHRJbnB1dFNlbmRSZXBsYWNlUmFuZ2UgcGFyYW1ldGVyCisgICAg ICAgIC8vIGZvciB0aGUga0V2ZW50Q2xhc3NUZXh0SW5wdXQva0V2ZW50VGV4dElucHV0VXBkYXRl QWN0aXZlSW5wdXRBcmVhIFRTTSBEb2MgQWNjZXNzIGV2ZW50LgorICAgICAgICAvLyBUaGUgQXBw S2l0IGFkZHMgYSAnc2VjcmV0JyBwcm9wZXJ0eSB0byB0aGUgc3RyaW5nIHRoYXQgY29udGFpbnMg dGhlIHJlcGxhY2VtZW50CisgICAgICAgIC8vIHJhbmdlLiAgVGhlIHJlcGxhY2VtZW50IHJhbmdl IGlzIHRoZSByYW5nZSBvZiB0aGUgdGV4dCB0aGF0IHNob3VsZCBiZSByZXBsYWNlZAorICAgICAg ICAvLyB3aXRoIHRoZSBuZXcgc3RyaW5nLgorICAgICAgICBOU1N0cmluZyAqcmFuZ2VTdHJpbmcg PSBbc3RyaW5nIGF0dHJpYnV0ZTpOU1RleHRJbnB1dFJlcGxhY2VtZW50UmFuZ2VBdHRyaWJ1dGVO YW1lIGF0SW5kZXg6MCBsb25nZXN0RWZmZWN0aXZlUmFuZ2U6TlVMTCBpblJhbmdlOk5TTWFrZVJh bmdlKDAsIFt0ZXh0IGxlbmd0aF0pXTsKKyAgICAgICAgaWYgKHJhbmdlU3RyaW5nKSB7CisgICAg ICAgICAgICBbW3NlbGYgX2JyaWRnZV0gc2VsZWN0TlNSYW5nZTpOU1JhbmdlRnJvbVN0cmluZyhy YW5nZVN0cmluZyldOworICAgICAgICB9CiAgICAgfSBlbHNlIHsKIAl0ZXh0ID0gc3RyaW5nOwog ICAgIH0K