Bug 55902

Summary:

selectionHasStyle and selectionStartHasStyle should use EditingStyle

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

HTML Editing

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Enhancement

CC:

darin, enrica, eric, ojan, tony

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

54932, 55904

Attachments:

Description	Flags
cleanup	none
cleanup 2	darin: review+

Description Ryosuke Niwa 2011-03-07 13:41:01 PST

We should deploy EditingStyle in selectionHasStyle and selectionStartHasStyle.

Comment 1 Ryosuke Niwa 2011-03-07 16:31:15 PST

Created attachment 84993 [details]
cleanup

Comment 2 Ryosuke Niwa 2011-03-07 16:43:30 PST

Created attachment 84994 [details]
cleanup 2

Comment 3 Ryosuke Niwa 2011-03-07 16:45:03 PST

Comment on attachment 84993 [details]
cleanup

View in context: https://bugs.webkit.org/attachment.cgi?id=84993&action=review

> Source/WebCore/page/ContextMenuController.cpp:41
> +#include "EditingStyle.h"

I got rid of this line but two patches are identical otherwise.

Comment 4 Darin Adler 2011-03-07 18:06:27 PST

Comment on attachment 84994 [details]
cleanup 2

View in context: https://bugs.webkit.org/attachment.cgi?id=84994&action=review

> Source/WebCore/editing/EditingStyle.cpp:516
> +// CSS properties that only has a visual difference when applied to text.

Verb agreement problem here. Should be “have” instead of “has”.

But I think the verb “create” is better.

> Source/WebCore/editing/EditingStyle.cpp:527
> +    RefPtr<CSSMutableStyleDeclaration> diff = getPropertiesNotIn(m_mutableStyle.get(), styleToCompare);

Even though diff is a “term of art”, I would use the word “difference” here in this code you moved.

Comment 5 Ryosuke Niwa 2011-03-07 18:46:26 PST

Thanks for the review as always!  Will fix both before landing the patch.

(In reply to comment #4)
> (From update of attachment 84994 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84994&action=review
> 
> > Source/WebCore/editing/EditingStyle.cpp:516
> > +// CSS properties that only has a visual difference when applied to text.
> 
> Verb agreement problem here. Should be “have” instead of “has”.
> 
> But I think the verb “create” is better.
> 
> > Source/WebCore/editing/EditingStyle.cpp:527
> > +    RefPtr<CSSMutableStyleDeclaration> diff = getPropertiesNotIn(m_mutableStyle.get(), styleToCompare);
> 
> Even though diff is a “term of art”, I would use the word “difference” here in this code you moved.

Comment 6 Ryosuke Niwa 2011-03-07 19:54:58 PST

Committed r80528: <http://trac.webkit.org/changeset/80528>