removeComputedInheritablePropertiesFrom is not used anywhere and copyInheritableProperties is used across the editing code but very inconsistently. copyInheritableProperties is poorly named considering the usage of the function. copyInheritableProperties should be deprecated and callers should use a better named function. Below are just examples of how copyInheritableProperties is used: 553@ReplaceSelectionCommand.cpp RefPtr<CSSMutableStyleDeclaration> styleAtInsertionPos = rangeCompliantEquivalent(insertionPos).computedStyle()->copyInheritableProperties(); String styleText = styleAtInsertionPos->cssText(); if (styleText == static_cast<Element*>(sourceDocumentStyleSpan)->getAttribute(styleAttr)) { Here, it's used to compare the styles of the source and destination. 55@RemoveFormatCommand.cpp // Get the default style for this editable root, it's the style that we'll give the // content that we're operating on. Node* root = frame->selection()->rootEditableElement(); RefPtr<CSSMutableStyleDeclaration> defaultStyle = computedStyle(root)->copyInheritableProperties(); Here, it's used to copy styles. 281@DeleteSelectionCommand.cpp // Figure out the typing style in effect before the delete is done. RefPtr<CSSComputedStyleDeclaration> computedStyle = positionBeforeTabSpan(m_selectionToDelete.start()).computedStyle(); m_typingStyle = computedStyle->copyInheritableProperties(); And here, it's used to retrieve the typing style.
Eric and I figured out that the last time any significant change was made to this function was http://trac.webkit.org/changeset/7314.
It seems to me that while there are several usages of this function, we can categorize it into two: 1. Preserving styles - e.g. you want to preserve styles while editing Because we almost always remove redundant styles when reapplying the style, we should make a function that takes the destination position, and applies the minimum styles automatically. 2. Typing styles - we set typingStyle A lot of problems are caused by typing styles (e.g. highlight is lost in various cases). It may make sense to make a new class, say, TypyingStyle class that centralizes code related to typing style. Any alternative ideas / suggestions?
Created attachment 32825 [details] removes removeComputedInheritablePropertiesFrom and renames copyInheritableProperties
Created attachment 32826 [details] Deprecates copyInheritableProperties and removes removeComputedInheritablePropertiesFrom Added more detailed comments
This seems reasonable except that preserving styles will still rely on the concept of inheritable properties. Will it just not use this method that you're deprecating?
(In reply to comment #5) > This seems reasonable except that preserving styles will still rely on the > concept of inheritable properties. Will it just not use this method that > you're deprecating? I think when we preserve style during editing operations, we also want to preserve background color and text decorations, which are not inheritable properties of CSS. So the name is still confusing.
Comment on attachment 32826 [details] Deprecates copyInheritableProperties and removes removeComputedInheritablePropertiesFrom Seems fine.
Landed in http://trac.webkit.org/changeset/46280