Bug 55902

Summary: selectionHasStyle and selectionStartHasStyle should use EditingStyle
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Enhancement CC: darin, enrica, eric, ojan, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 54932, 55904    
Attachments:
Description Flags
cleanup
none
cleanup 2 darin: review+

Ryosuke Niwa
Reported 2011-03-07 13:41:01 PST
We should deploy EditingStyle in selectionHasStyle and selectionStartHasStyle.
Attachments
cleanup (22.72 KB, patch)
2011-03-07 16:31 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
cleanup 2 (22.53 KB, patch)
2011-03-07 16:43 PST, Ryosuke Niwa 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2011-03-07 16:31:15 PST
Created attachment 84993 [details] cleanup
Ryosuke Niwa
Comment 2 2011-03-07 16:43:30 PST
Created attachment 84994 [details] cleanup 2
Ryosuke Niwa
Comment 3 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.
Darin Adler
Comment 4 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.
Ryosuke Niwa
Comment 5 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.
Ryosuke Niwa
Comment 6 2011-03-07 19:54:58 PST
Committed r80528: <http://trac.webkit.org/changeset/80528>
Note You need to log in before you can comment on or make changes to this bug.