[webkit-reviews] review denied: [Bug 121791] ASSERTION FAILED: comparePositions(newEnd, newStart) >= 0 in WebCore::ApplyStyleCommand::updateStartEnd : [Attachment 213064] Proposed patch

[bugzilla-daemon at webkit.org]

Andreas Kling <akling at apple.com> has denied Renata Hodovan <reni at webkit.org>'s
request for review:
Bug 121791: ASSERTION FAILED: comparePositions(newEnd, newStart) >= 0 in
WebCore::ApplyStyleCommand::updateStartEnd
https://bugs.webkit.org/show_bug.cgi?id=121791

Attachment 213064: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=213064&action=review

------- Additional Comments from Andreas Kling <akling at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=213064&action=review


> LayoutTests/ChangeLog:8
> +	   * editing/execCommand/remove-formatting-from-iframe-in-button.html:
Added.

This should be a dumpAsText test.

> LayoutTests/ChangeLog:9
> +	   *
platform/qt/editing/execCommand/remove-formatting-from-iframe-in-button-expecte
d.txt: Added.

There is no Qt port anymore.

> Source/WebCore/editing/TextIterator.cpp:2433
>	       // FIXME: This is a workaround for the fact that the end of a
run is often at the wrong
>	       // position for emitted '\n's.
> -	       if (len == 1 && it.characterAt(0) == '\n') {
> +	       if (len == 1 && (it.characterAt(0) == '\n' || it.characterAt(0)
== ',')) {

I don't know this code well enough to tell if the change is correct, but it
looks to me like you are bringing the code out of sync with the preceding
FIXME.