Summary: | No selection notification for editing operation that doesn't change the selection's DOM position | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Justin Garcia <justin.garcia> | ||||||
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, enrica, eric, ojan, tony, webkit.review.bot | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Justin Garcia
2010-05-26 16:43:26 PDT
Created attachment 57273 [details]
patch
Patch. Layout test results for qt and chromium-win will also need to be updated.
Justin and I discussed this at length over irc. The patch looks good to me. Just fix the typo in th change log and in the comments in the code. 'effecting' --> 'affecting' Maybe Ojan can r+? I don't understand this patch too well. Doesn't this mean that we call respondToChangedSelection for every Command we apply? Wouldn't that cause us to start a new kill ring sequence too often (i.e. when the command didn't actually cause the selection to change)? Also, is there a way to test this, e.g. the rtl/ltr case below? The operations "change" the selection, but the fact that the operations don't *change the selection's location in the DOM* is just an implementation detail. Note for example that RTL -> LTR *will* result in a call to respondToChangedSelection() if the DOM was: <div contentEditable="true" style="direction: RTL">^<br></div> since a new div will need to be created to give the paragraph direction:LTR. This will change the DOM enough to change the selection's DOM location after the operation. I don't see any reason why we should start a new kill ring sequence for this DOM but not the one mentioned in the bug. Same for the change notification. Created attachment 58824 [details] patch > Also, is there a way to test this, e.g. the rtl/ltr case below? Not exactly, but editing/style/block-style-001.html demonstrates the bug during a change in text-alignment, which is basically the same thing. New patch mentions this and fixes the typo that Enrica found. Comment on attachment 58824 [details]
patch
WebCore/editing/Editor.cpp:2894
+ // For example when you press return in the following (the caret is marked by ^):
s/effecting/affecting
http://trac.webkit.org/changeset/61418 might have broken Qt Linux Release |