Bug 45616

Summary:

applyInlineStyleToNodeRange does not extend a run properly

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

HTML Editing

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Blocker

CC:

abarth, darin, enrica, eric, ojan, tkent, tony, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

45746

Bug Blocks:

45568

Attachments:

Description	Flags
fixes the bug	none
fixed per Tony's comment	tony: review+

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	Details Formatted Diff Diff
fixed per Tony's comment (21.75 KB, patch) 2010-09-13 14:24 PDT, Ryosuke Niwa	tony: review+	Details Formatted Diff Diff
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.