Bug 27476 – execCommand('underline' / 'strikeThrough') doesn't work properly with multiple styles in text-decoration

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Ryosuke Niwa]

execCommand('underline' / 'strikeThrough') results in text-decoration: underline / line-through, deleting all other text decorations even when more than one styles are specified.

[Ryosuke Niwa]

Created attachment 33278 [details]
fixes the bug

[Eric Seidel (no email)]

Comment on attachment 33278 [details]
fixes the bug

/CSSComputedStyleDeclaration.cpp change needs testing.
window.getComputedStyle(foo).getPropertyCSSValue('text-decoration')


   RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
 1055             RefPtr<CSSValueList> list = CSSValueList::createSpaceSeparated();
needs test too.
foo.style.textDecoration

 73 void CSSValueList::remove(CSSValue* val)
should be called removeFirst or removeAll or should at least be documented as to its behavior.

 78         if (m_values.at(index)->cssText() == val->cssText())

is really really sad.

consider adding a bool return to remove and then you don't need hasValue

 93 static bool applyCommandToFrame(Frame* frame, EditorCommandSource source, EditAction action, CSSMutableStyleDeclaration* style)
is a great change and could even be made in a separate patch.  it's OK to do in this one though.

YOu can just delete:
110116 static bool executeApplyStyle(Frame* frame, EditorCommandSource source, EditAction action, int propertyID, const char* propertyValue)

Spacing:
 130     Node* nodeToRemove=0;

This looks great!  We just need more tests!

[Ryosuke Niwa]

Created attachment 33304 [details]
2nd submission, no longer adds hasValue to CSSValueList

[Eric Seidel (no email)]

Comment on attachment 33304 [details]
2nd submission, no longer adds hasValue to CSSValueList

getComputedStyle/getComputedStyle-text-decoration.html shoudl be a js test (like how editing/execCommand/resources/toggle-text-decorations.js is).  Ping me if you need help converting it.

missing newline at EOF:
29 var successfullyParsed = true;
030 \ No newline at end of file

Would like to see the converted test. Otherwise this looks fine.

[Ryosuke Niwa]

Created attachment 33308 [details]
3rd submission, test for computed style is converted to a js test

[Eric Seidel (no email)]

Comment on attachment 33308 [details]
3rd submission, test for computed style is converted to a js test

Looks fine.

[Ryosuke Niwa]

Landed in http://trac.webkit.org/changeset/46251