ApplyStyleCommand::applyInlineStyle modifies the EditingStyle by calling collapseTextDecorationProperties but it shouldn't ApplyStyleCommand should be unaware of crazy text decoration manipulations we do in EditingStyle.
Created attachment 95684 [details] refactoring
This will remove the last trace of text-decoration / -webkit-text-decorations-in-effect from ApplyStyleCommand. And in fact, there will be no mentioning of these properties in ApplyStyleCommand after this patch! ApplyStyleCommand still knows about font-size, unicode-bidi, and direction properties but hopefully I can get rid of them soon as well.
Comment on attachment 95684 [details] refactoring View in context: https://bugs.webkit.org/attachment.cgi?id=95684&action=review > Source/WebCore/editing/EditingStyle.cpp:609 > + if (propertyID == CSSPropertyWebkitTextDecorationsInEffect && inlineStyle->getPropertyCSSValue(CSSPropertyTextDecoration)) { > + if (!conflictingProperties) > return true; > + conflictingProperties->append(CSSPropertyTextDecoration); > + if (extractedStyle) > + extractedStyle->setProperty(CSSPropertyTextDecoration, inlineStyle->getPropertyValue(CSSPropertyTextDecoration), inlineStyle->getPropertyPriority(CSSPropertyTextDecoration)); > + continue; Just to clarify, the only code change I'm making here is these lines where I explicitly add text-decoration property to the list of conflicting properties when I see -webkit-text-decorations-in-effect in m_mutableStyle and inlineStyle has text-decoration property. Because inlineStyle is element's inline style declaration, we don't have to consider the case where inlineStyle has -webkit-text-decorations-in-effect property. The rest of changes are due to my merging two loops together. Whereas we used to have a faster loop when conflictingProperties == 0, I merged them and added return true when !conflictingProperties in several places.
Comment on attachment 95684 [details] refactoring View in context: https://bugs.webkit.org/attachment.cgi?id=95684&action=review > Source/WebCore/editing/EditingStyle.cpp:171 > + RefPtr<CSSValue> styleValue = style->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect); Could you explain why you don't use m_propertyID anymore? It's not clear to me.
(In reply to comment #4) > (From update of attachment 95684 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=95684&action=review > > > Source/WebCore/editing/EditingStyle.cpp:171 > > + RefPtr<CSSValue> styleValue = style->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect); > > Could you explain why you don't use m_propertyID anymore? It's not clear to me. I have to test for both text-decoration and -webkit-text-decorations-in-effect because both of these property should match u, s, & strike.
Comment on attachment 95684 [details] refactoring Looks good to me
Comment on attachment 95684 [details] refactoring Clearing flags on attachment: 95684 Committed r87952: <http://trac.webkit.org/changeset/87952>
All reviewed patches have been landed. Closing bug.