RESOLVED FIXED45616
applyInlineStyleToNodeRange does not extend a run properly 
https://bugs.webkit.org/show_bug.cgi?id=45616
Summary applyInlineStyleToNodeRange does not extend a run properly
Ryosuke Niwa
Reported 2010-09-12 10:27:50 PDT
In the current implementation of applyInlineStyleToNodeRange, we do not start a run from a node that contains child nodes. We also fail to remove redundant styled elements in a run.
Attachments
fixes the bug (21.61 KB, patch)
2010-09-12 10:54 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
fixed per Tony's comment (21.75 KB, patch)
2010-09-13 14:24 PDT, Ryosuke Niwa 		tony: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2010-09-12 10:54:02 PDT
Created attachment 67338 [details] fixes the bug
Tony Chang
Comment 2 2010-09-13 11:59:09 PDT
Comment on attachment 67338 [details] fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=67338&action=prettypatch > WebCore/editing/ApplyStyleCommand.cpp:1170 > - if (mode == RemoveAttributesAndElements) > + if (mode == RemoveIfNeeded) > removeNodePreservingChildren(element); Should mode == RemoveAlways also cause us to removeNodePreservingChildren?
Ryosuke Niwa
Comment 3 2010-09-13 13:39:41 PDT
(In reply to comment #2) > (From update of attachment 67338 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=67338&action=prettypatch > > > WebCore/editing/ApplyStyleCommand.cpp:1170 > > - if (mode == RemoveAttributesAndElements) > > + if (mode == RemoveIfNeeded) > > removeNodePreservingChildren(element); > Should mode == RemoveAlways also cause us to removeNodePreservingChildren? That's a very valid point. Let me fix that.
Ryosuke Niwa
Comment 4 2010-09-13 14:24:42 PDT
Created attachment 67471 [details] fixed per Tony's comment
Ryosuke Niwa
Comment 5 2010-09-14 00:12:13 PDT
Committed r67449: <http://trac.webkit.org/changeset/67449>
WebKit Review Bot
Comment 6 2010-09-14 01:01:41 PDT
http://trac.webkit.org/changeset/67449 might have broken Qt Linux Release minimal The following changes are on the blame list: http://trac.webkit.org/changeset/67449 http://trac.webkit.org/changeset/67450 http://trac.webkit.org/changeset/67451
Ryosuke Niwa
Comment 7 2010-09-14 07:39:12 PDT
http://trac.webkit.org/changeset/67459 rolled out this patch due the failure of /fast/events/event-input-contentEditable.html Will rebaseline this patch and commit again.
Eric Seidel (no email)
Comment 8 2010-09-14 13:04:25 PDT
I can't tell who was to blame, I thought bug 45071 was what caused that test to regress?
Ryosuke Niwa
Comment 9 2010-09-14 13:29:15 PDT
Committed r67490: <http://trac.webkit.org/changeset/67490>
Note You need to log in before you can comment on or make changes to this bug.