Summary: | -[WebHTMLView insertText:] needs to support NSTextInputReplacementRangeAttributeName | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Evan Gross <evan> | ||||
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED DUPLICATE | ||||||
Severity: | Normal | CC: | ap, ian, oliver | ||||
Priority: | P2 | ||||||
Version: | 420+ | ||||||
Hardware: | Mac | ||||||
OS: | OS X 10.4 | ||||||
Attachments: |
|
Description
Evan Gross
2005-08-26 23:20:19 PDT
Created attachment 3874 [details]
proposed patch
Sounds like a good patch, but how to test this? What steps with what input method will demonstrate it? Comment on attachment 3874 [details]
proposed patch
The patch looks good but r- for lack of test case or even steps to reproduce.
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). Comment on attachment 3874 [details]
proposed patch
Putting back in review? since steps to reproduce are now provided
Comment on attachment 3874 [details]
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?
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). Comment on attachment 3874 [details]
proposed patch
OK, lets work on a version of this that gets as many as possible of the fine
points Evan mentions working.
*** This bug has been marked as a duplicate of 13664 *** Although fixing this issue would hide the symptoms of bug 13664, they are not duplicates (see bug 13664 comment 19). 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. Whoops, should be resolved duplicate re-duplicating *** This bug has been marked as a duplicate of 13664 *** |