Suppose I do an editing operation that changes: <div style="rtl">^<br></div> to <div style="ltr">^<br></div>. ApplyStyleCommand will just change the inline style declaration of the div without otherwise touching the DOM, so there is no selection change notification, but we need one because it changes visually (the caret moves from the right to the left).
Created attachment 57273 [details]
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]
> 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]
+ // For example when you press return in the following (the caret is marked by ^):
http://trac.webkit.org/changeset/61418 might have broken Qt Linux Release