Fix for this bug:
<rdar://problem/
3751098> HTML email has one set of SPAN tags per character in the message
Progress on this bug:
<rdar://problem/
3755562> Typing styles do not use same tag application conventions as font and color panel
* khtml/editing/htmlediting_impl.cpp:
(khtml::CompositeEditCommandImpl::applyTypingStyle): Name changed from createTypingStyleElement.
Also, interface changed to take the node to which the typing style is to be applied.
This makes it easier to apply what may be up to three levels of nested tags to get the
desired style (<B>, <I>, and <SPAN STYLE="">).
Also, Borrow some of the style change smarts from ApplyStyleCommandImpl to use bold and
italic tags for applying styles when that is apprpriate. This creates on opportunity to
factor the code to do this so that this function and the ApplyStyleCommandImpl class can
share the implementation. I will follow up with a change to do that after landing this
change. Some future code factoring could be done here to bring together some similar code
into one place.
(khtml::ApplyStyleCommandImpl::applyStyleIfNeeded): Add comment about code factoring work.
(khtml::ApplyStyleCommandImpl::computeStyleChange): StyleChange struct no longer a member of the
ApplyStyleCommandImpl class. CompositeEditCommandImpl needs it now in its applyTypingStyle()
function.
(khtml::InputNewlineCommandImpl::doApply): Pass along node to style to applyTypingStyle.
(khtml::InputTextCommandImpl::prepareForTextInsertion): Ditto.
* khtml/editing/htmlediting_impl.h:
(khtml::StyleChange::StyleChange): Pull this struct out of ApplyStyleCommandImpl so
CompositeEditCommandImpl can use it.
* khtml/khtml_part.cpp:
(KHTMLPart::notifySelectionChanged): Always clear typing style when the selection
changes, not only when closing typing. This fixes
3751098.
These three tests actually had results that treated the buggy behavior as correct!
* layout-tests/editing/style/style-
3681552-fix-001-expected.txt
* layout-tests/editing/style/style-
3681552-fix-002-expected.txt
* layout-tests/editing/style/typing-style-002-expected.txt
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@7240
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
RenderInline {SPAN} at (0,0) size 157x28
RenderText {TEXT} at (14,14) size 70x28
text run at (14,14) width 70: "here is "
- RenderInline {SPAN} at (0,0) size 44x28
- RenderText {TEXT} at (84,14) size 11x28
- text run at (84,14) width 11: "x"
- RenderInline {SPAN} at (0,0) size 33x28
- RenderText {TEXT} at (95,14) size 11x28
- text run at (95,14) width 11: "x"
- RenderInline {SPAN} at (0,0) size 22x28
- RenderText {TEXT} at (106,14) size 11x28
- text run at (106,14) width 11: "x"
- RenderInline {SPAN} at (0,0) size 11x28
- RenderText {TEXT} at (117,14) size 11x28
- text run at (117,14) width 11: "x"
+ RenderInline {I} at (0,0) size 44x28
+ RenderText {TEXT} at (84,14) size 44x28
+ text run at (84,14) width 44: "xxxx"
RenderText {TEXT} at (128,14) size 43x28
text run at (128,14) width 43: " text"
RenderText {TEXT} at (0,0) size 0x0
selection is CARET:
-start: position 1 of child 1 {TEXT} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of root {DIV}
-upstream: position 1 of child 1 {TEXT} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of root {DIV}
+start: position 4 of child 1 {TEXT} of child 2 {I} of child 2 {SPAN} of root {DIV}
+upstream: position 4 of child 1 {TEXT} of child 2 {I} of child 2 {SPAN} of root {DIV}
downstream: position 0 of child 3 {TEXT} of child 2 {SPAN} of root {DIV}
RenderBlock {HTML} at (0,0) size 800x600
RenderBody {BODY} at (8,8) size 784x584
RenderBlock {DIV} at (0,0) size 784x56 [border: (2px solid #FF0000)]
- RenderInline {SPAN} at (0,0) size 157x28
- RenderText {TEXT} at (14,14) size 70x28
- text run at (14,14) width 70: "here is "
- RenderInline {SPAN} at (0,0) size 44x28
- RenderText {TEXT} at (84,14) size 11x28
- text run at (84,14) width 11: "x"
- RenderInline {SPAN} at (0,0) size 33x28
- RenderText {TEXT} at (95,14) size 11x28
- text run at (95,14) width 11: "x"
- RenderInline {SPAN} at (0,0) size 22x28
- RenderText {TEXT} at (106,14) size 11x28
- text run at (106,14) width 11: "x"
- RenderInline {SPAN} at (0,0) size 11x28
- RenderText {TEXT} at (117,14) size 11x28
- text run at (117,14) width 11: "x"
- RenderText {TEXT} at (128,14) size 43x28
- text run at (128,14) width 43: " text"
+ RenderInline {SPAN} at (0,0) size 161x28
+ RenderText {TEXT} at (14,14) size 118x28
+ text run at (14,14) width 118: "here is xxxx"
+ RenderText {TEXT} at (132,14) size 43x28
+ text run at (132,14) width 43: " text"
RenderText {TEXT} at (0,0) size 0x0
selection is CARET:
-start: position 1 of child 1 {TEXT} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of root {DIV}
-upstream: position 1 of child 1 {TEXT} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of root {DIV}
-downstream: position 0 of child 3 {TEXT} of child 2 {SPAN} of root {DIV}
+start: position 12 of child 1 {TEXT} of child 2 {SPAN} of root {DIV}
+upstream: position 12 of child 1 {TEXT} of child 2 {SPAN} of root {DIV}
+downstream: position 0 of child 2 {TEXT} of child 2 {SPAN} of root {DIV}
RenderText {TEXT} at (14,14) size 70x28
text run at (14,14) width 70: "here is "
RenderInline {I} at (0,0) size 53x28
- RenderText {TEXT} at (84,14) size 9x28
- text run at (84,14) width 9: "s"
- RenderInline {SPAN} at (0,0) size 44x28
- RenderText {TEXT} at (93,14) size 11x28
- text run at (93,14) width 11: "x"
- RenderInline {SPAN} at (0,0) size 33x28
- RenderText {TEXT} at (104,14) size 11x28
- text run at (104,14) width 11: "x"
- RenderInline {SPAN} at (0,0) size 22x28
- RenderText {TEXT} at (115,14) size 11x28
- text run at (115,14) width 11: "x"
- RenderInline {SPAN} at (0,0) size 11x28
- RenderText {TEXT} at (126,14) size 11x28
- text run at (126,14) width 11: "x"
+ RenderText {TEXT} at (84,14) size 53x28
+ text run at (84,14) width 53: "sxxxx"
RenderText {TEXT} at (137,14) size 43x28
text run at (137,14) width 43: " text"
RenderText {TEXT} at (0,0) size 0x0
selection is CARET:
-start: position 1 of child 1 {TEXT} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of child 2 {I} of child 2 {SPAN} of root {DIV}
-upstream: position 1 of child 1 {TEXT} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of child 2 {SPAN} of child 2 {I} of child 2 {SPAN} of root {DIV}
+start: position 5 of child 1 {TEXT} of child 2 {I} of child 2 {SPAN} of root {DIV}
+upstream: position 5 of child 1 {TEXT} of child 2 {I} of child 2 {SPAN} of root {DIV}
downstream: position 0 of child 3 {TEXT} of child 2 {SPAN} of root {DIV}
+2004-08-12 Ken Kocienda <kocienda@apple.com>
+
+ Reviewed by Darin
+
+ Fix for this bug:
+ <rdar://problem/3751098> HTML email has one set of SPAN tags per character in the message
+
+ Progress on this bug:
+ <rdar://problem/3755562> Typing styles do not use same tag application conventions as font and color panel
+
+ * khtml/editing/htmlediting_impl.cpp:
+ (khtml::CompositeEditCommandImpl::applyTypingStyle): Name changed from createTypingStyleElement.
+ Also, interface changed to take the node to which the typing style is to be applied.
+ This makes it easier to apply what may be up to three levels of nested tags to get the
+ desired style (<B>, <I>, and <SPAN STYLE="">).
+ Also, Borrow some of the style change smarts from ApplyStyleCommandImpl to use bold and
+ italic tags for applying styles when that is apprpriate. This creates on opportunity to
+ factor the code to do this so that this function and the ApplyStyleCommandImpl class can
+ share the implementation. I will follow up with a change to do that after landing this
+ change. Some future code factoring could be done here to bring together some similar code
+ into one place.
+ (khtml::ApplyStyleCommandImpl::applyStyleIfNeeded): Add comment about code factoring work.
+ (khtml::ApplyStyleCommandImpl::computeStyleChange): StyleChange struct no longer a member of the
+ ApplyStyleCommandImpl class. CompositeEditCommandImpl needs it now in its applyTypingStyle()
+ function.
+ (khtml::InputNewlineCommandImpl::doApply): Pass along node to style to applyTypingStyle.
+ (khtml::InputTextCommandImpl::prepareForTextInsertion): Ditto.
+ * khtml/editing/htmlediting_impl.h:
+ (khtml::StyleChange::StyleChange): Pull this struct out of ApplyStyleCommandImpl so
+ CompositeEditCommandImpl can use it.
+ * khtml/khtml_part.cpp:
+ (KHTMLPart::notifySelectionChanged): Always clear typing style when the selection
+ changes, not only when closing typing. This fixes 3751098.
+
+ These three tests actually had results that treated the buggy behavior as correct!
+
+ * layout-tests/editing/style/style-3681552-fix-001-expected.txt
+ * layout-tests/editing/style/style-3681552-fix-002-expected.txt
+ * layout-tests/editing/style/typing-style-002-expected.txt
+
2004-08-12 Darin Adler <darin@apple.com>
Reviewed by Ken.
applyCommandToComposite(cmd);
}
-ElementImpl *CompositeEditCommandImpl::createTypingStyleElement() const
+ElementImpl *CompositeEditCommandImpl::applyTypingStyle(NodeImpl *child) const
{
- int exceptionCode = 0;
- ElementImpl *styleElement = document()->createHTMLElement("SPAN", exceptionCode);
- ASSERT(exceptionCode == 0);
+ // FIXME: This function should share code with ApplyStyleCommandImpl::applyStyleIfNeeded
+ // and ApplyStyleCommandImpl::computeStyleChange.
+ // Both function do similar work, and the common parts could be factored out.
+
+ CSSStyleDeclarationImpl *style = document()->part()->typingStyle();
+ StyleChange styleChange;
+
+ for (QPtrListIterator<CSSProperty> it(*(style->values())); it.current(); ++it) {
+ CSSProperty *property = it.current();
+ switch (property->id()) {
+ case CSS_PROP_FONT_WEIGHT:
+ if (strcasecmp(property->value()->cssText(), "bold") == 0)
+ styleChange.applyBold = true;
+ else
+ styleChange.cssStyle += property->cssText();
+ break;
+ case CSS_PROP_FONT_STYLE: {
+ DOMString cssText(property->value()->cssText());
+ if (strcasecmp(cssText, "italic") == 0 || strcasecmp(cssText, "oblique") == 0)
+ styleChange.applyItalic = true;
+ else
+ styleChange.cssStyle += property->cssText();
+ }
+ break;
+ default:
+ styleChange.cssStyle += property->cssText();
+ break;
+ }
+ }
- styleElement->setAttribute(ATTR_STYLE, document()->part()->typingStyle()->cssText().implementation(), exceptionCode);
- ASSERT(exceptionCode == 0);
+ NodeImpl *childToAppend = child;
+ ElementImpl *element = 0;
+ int exceptionCode = 0;
- styleElement->setAttribute(ATTR_CLASS, styleSpanClassString());
- ASSERT(exceptionCode == 0);
+ if (styleChange.applyItalic) {
+ ElementImpl *italicElement = document()->createHTMLElement("I", exceptionCode);
+ ASSERT(exceptionCode == 0);
+ italicElement->appendChild(childToAppend, exceptionCode);
+ ASSERT(exceptionCode == 0);
+ element = italicElement;
+ childToAppend = italicElement;
+ }
+
+ if (styleChange.applyBold) {
+ ElementImpl *boldElement = document()->createHTMLElement("B", exceptionCode);
+ ASSERT(exceptionCode == 0);
+ boldElement->appendChild(childToAppend, exceptionCode);
+ ASSERT(exceptionCode == 0);
+ element = boldElement;
+ childToAppend = boldElement;
+ }
- return styleElement;
+ if (styleChange.cssStyle.length() > 0) {
+ ElementImpl *styleElement = document()->createHTMLElement("SPAN", exceptionCode);
+ ASSERT(exceptionCode == 0);
+ styleElement->setAttribute(ATTR_STYLE, styleChange.cssStyle);
+ styleElement->setAttribute(ATTR_CLASS, styleSpanClassString());
+ styleElement->appendChild(childToAppend, exceptionCode);
+ ASSERT(exceptionCode == 0);
+ element = styleElement;
+ childToAppend = styleElement;
+ }
+
+ return element;
}
//==========================================================================================
void ApplyStyleCommandImpl::applyStyleIfNeeded(NodeImpl *startNode, NodeImpl *endNode)
{
+ // FIXME: This function should share code with CompositeEditCommandImpl::applyTypingStyle.
+ // Both function do similar work, and the common parts could be factored out.
+
StyleChange styleChange = computeStyleChange(Position(startNode, 0), style());
int exceptionCode = 0;
return strcasecmp(value->cssText(), property->value()->cssText()) == 0;
}
-ApplyStyleCommandImpl::StyleChange ApplyStyleCommandImpl::computeStyleChange(const Position &insertionPoint, CSSStyleDeclarationImpl *style)
+StyleChange ApplyStyleCommandImpl::computeStyleChange(const Position &insertionPoint, CSSStyleDeclarationImpl *style)
{
ASSERT(insertionPoint.notEmpty());
ASSERT(style);
// Handle the case where there is a typing style.
CSSStyleDeclarationImpl *typingStyle = document()->part()->typingStyle();
- if (typingStyle && typingStyle->length() > 0) {
- ElementImpl *styleElement = createTypingStyleElement();
- styleElement->appendChild(breakNode, exceptionCode);
- ASSERT(exceptionCode == 0);
- nodeToInsert = styleElement;
- }
+ if (typingStyle && typingStyle->length() > 0)
+ nodeToInsert = applyTypingStyle(breakNode);
Position pos(selection.start().equivalentDownstreamPosition());
bool atStart = pos.offset() <= pos.node()->caretMinOffset();
// Handle the case where there is a typing style.
CSSStyleDeclarationImpl *typingStyle = document()->part()->typingStyle();
- if (typingStyle && typingStyle->length() > 0) {
- int exceptionCode = 0;
- ElementImpl *styleElement = createTypingStyleElement();
- styleElement->appendChild(textNode, exceptionCode);
- ASSERT(exceptionCode == 0);
- nodeToInsert = styleElement;
- }
+ if (typingStyle && typingStyle->length() > 0)
+ nodeToInsert = applyTypingStyle(textNode);
// Now insert the node in the right place
if (pos.node()->isEditableBlock()) {
setEndingSelection(Position(cmd.node(), 0));
}
- int exceptionCode = 0;
TextImpl *editingTextNode = document()->createEditingTextNode("");
-
- ElementImpl *styleElement = createTypingStyleElement();
- styleElement->appendChild(editingTextNode, exceptionCode);
- ASSERT(exceptionCode == 0);
-
NodeImpl *node = endingSelection().start().equivalentUpstreamPosition().node();
- insertNodeAfter(styleElement, node);
+ insertNodeAfter(applyTypingStyle(editingTextNode), node);
pos = Position(editingTextNode, 0);
}
}
namespace khtml {
+//------------------------------------------------------------------------------------------
+// StyleChange
+
+struct StyleChange {
+ StyleChange() : applyBold(false), applyItalic(false) {}
+ DOM::DOMString cssStyle;
+ bool applyBold;
+ bool applyItalic;
+};
+
//------------------------------------------------------------------------------------------
// EditCommandImpl
void setNodeAttribute(DOM::ElementImpl *, int attribute, const DOM::DOMString &);
void splitTextNode(DOM::TextImpl *text, long offset);
- DOM::ElementImpl *createTypingStyleElement() const;
+ DOM::ElementImpl *applyTypingStyle(DOM::NodeImpl *) const;
QValueList<EditCommand> m_cmds;
};
DOM::CSSStyleDeclarationImpl *style() const { return m_style; }
- struct StyleChange {
- StyleChange() : applyBold(false), applyItalic(false) {}
- DOM::DOMString cssStyle;
- bool applyBold:1;
- bool applyItalic:1;
- };
-
private:
// style-removal helpers
bool isHTMLStyleNode(DOM::HTMLElementImpl *);
{
selectionLayoutChanged();
- if (closeTyping) {
+ if (closeTyping)
TypingCommand::closeTyping(lastEditCommand());
- clearTypingStyle();
- }
+
+ clearTypingStyle();
emitSelectionChanged();
git://git.webkit.org / WebKit-https.git / commitdiff