LayoutTests:
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Jul 2007 00:04:19 +0000 (00:04 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Jul 2007 00:04:19 +0000 (00:04 +0000)
        Reviewed by Tim Hatcher.

        - tests for <rdar://problem/5221297> Font family name in the cssText for a DOMCSSStyleDeclaration needs quotes

        * editing/style/font-family-with-space-expected.txt: Added.
        * editing/style/font-family-with-space.html: Added.

        * fast/inspector/style-expected.txt: Updated expected results to expect "Lucida Grande" in quote marks.

WebCore:

        Reviewed by Tim Hatcher.

        - fix <rdar://problem/5221297> Font family name in the cssText for a DOMCSSStyleDeclaration needs quotes

        Test: editing/style/font-family-with-space.html

        * css/FontFamilyValue.cpp:
        (WebCore::isValidCSSIdentifier): Added. Implements the same rule that the CSS lexer does.
        (WebCore::quoteStringIfNeeded): Changed to call isValidCSSIdentifier instead of just
        checking for a leading "#" character.
        * editing/markup.cpp:
        (WebCore::escapeTextForMarkup): Changed to take a String parameter for better efficiency.
        (WebCore::renderedText): Changed to return a String for better efficiency.
        (WebCore::addNamespace): Updated to pass String to escapeTextForMarkup.
        (WebCore::startMarkup): Updated to pass String to escapeTextForMarkup. Added missing call
        to escapeTextForMarkup in the special case for the style property.
        (WebCore::createMarkup): Changed from single quotes to double quotes and also added missing
        call to escapeTextForMarkup in two special cases for the style property.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@23969 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/style/font-family-with-space-expected.txt [new file with mode: 0644]
LayoutTests/editing/style/font-family-with-space.html [new file with mode: 0644]
LayoutTests/fast/inspector/style-expected.txt
WebCore/ChangeLog
WebCore/css/FontFamilyValue.cpp
WebCore/editing/markup.cpp

index 47f52699acfc95392342690f4b948f363e142451..0a976e5dd830d658941dec697aaf5f00d937dfa3 100644 (file)
@@ -1,3 +1,14 @@
+2007-07-03  Darin Adler  <darin@apple.com>
+
+        Reviewed by Tim Hatcher.
+
+        - tests for <rdar://problem/5221297> Font family name in the cssText for a DOMCSSStyleDeclaration needs quotes
+
+        * editing/style/font-family-with-space-expected.txt: Added.
+        * editing/style/font-family-with-space.html: Added.
+
+        * fast/inspector/style-expected.txt: Updated expected results to expect "Lucida Grande" in quote marks.
+
 2007-07-03  Adele Peterson  <adele@apple.com>
 
         Reviewed by Darin.
diff --git a/LayoutTests/editing/style/font-family-with-space-expected.txt b/LayoutTests/editing/style/font-family-with-space-expected.txt
new file mode 100644 (file)
index 0000000..f8cab3d
--- /dev/null
@@ -0,0 +1,16 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderInline {SPAN} at (0,0) size 245x15
+        RenderText {#text} at (0,0) size 245x15
+          text run at (0,0) width 245: "This text should be Times New Roman bold."
+      RenderInline {SPAN} at (0,0) size 245x15
+        RenderInline {SPAN} at (0,0) size 245x15
+          RenderInline {DIV} at (0,0) size 245x15
+            RenderText {#text} at (245,0) size 245x15
+              text run at (245,0) width 245: "This text should be Times New Roman bold."
+      RenderText {#text} at (0,0) size 0x0
+      RenderText {#text} at (0,0) size 0x0
+caret: position 41 of child 0 {#text} of child 0 {DIV} of child 1 {SPAN} of child 0 {SPAN} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/style/font-family-with-space.html b/LayoutTests/editing/style/font-family-with-space.html
new file mode 100644 (file)
index 0000000..59230a7
--- /dev/null
@@ -0,0 +1,11 @@
+<html style="font-family: Arial; font-size: 13px;">
+<body contenteditable="true" style="font-family: 'Times New Roman'; font-weight: bold;">This text should be Times New Roman bold.</body>
+<script>
+document.body.focus();
+document.execCommand("SelectAll");
+document.execCommand("Underline");
+document.execCommand("Copy");
+window.getSelection().modify("move", "forward", "character");
+document.execCommand("Paste");
+</script>
+</html>
index 8411de51ffe8e86e80b24f90090f82d4a9e50808..73b57393c75857387ae9c5006a019a57870fbc03 100644 (file)
@@ -49,6 +49,6 @@ layer at (0,0) size 800x600
         RenderText {#text} at (0,216) size 75x18
           text run at (0,216) width 75: "color: white"
         RenderBR {BR} at (75,230) size 0x0
-        RenderText {#text} at (0,234) size 356x18
-          text run at (0,234) width 356: "font: normal normal normal 24px/normal Lucida Grande"
-        RenderBR {BR} at (356,248) size 0x0
+        RenderText {#text} at (0,234) size 362x18
+          text run at (0,234) width 362: "font: normal normal normal 24px/normal 'Lucida Grande'"
+        RenderBR {BR} at (362,248) size 0x0
index 3e10b09765d35ccf9dcc03e5cedc5801676be32f..c2aa82d85b0110a79c258e03f11a005c852fa0c1 100644 (file)
@@ -1,3 +1,24 @@
+2007-07-03  Darin Adler  <darin@apple.com>
+
+        Reviewed by Tim Hatcher.
+
+        - fix <rdar://problem/5221297> Font family name in the cssText for a DOMCSSStyleDeclaration needs quotes
+
+        Test: editing/style/font-family-with-space.html
+
+        * css/FontFamilyValue.cpp:
+        (WebCore::isValidCSSIdentifier): Added. Implements the same rule that the CSS lexer does.
+        (WebCore::quoteStringIfNeeded): Changed to call isValidCSSIdentifier instead of just
+        checking for a leading "#" character.
+        * editing/markup.cpp:
+        (WebCore::escapeTextForMarkup): Changed to take a String parameter for better efficiency.
+        (WebCore::renderedText): Changed to return a String for better efficiency.
+        (WebCore::addNamespace): Updated to pass String to escapeTextForMarkup.
+        (WebCore::startMarkup): Updated to pass String to escapeTextForMarkup. Added missing call
+        to escapeTextForMarkup in the special case for the style property.
+        (WebCore::createMarkup): Changed from single quotes to double quotes and also added missing
+        call to escapeTextForMarkup in two special cases for the style property.
+
 2007-07-03  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Darin.
index 2e6b382dcd4fe697666c67e3ab66bc5364880c59..fb1056280e9ec6f634495b5bea6fc47c96859943 100644 (file)
@@ -1,8 +1,6 @@
 /**
- * This file is part of the DOM implementation for KDE.
- *
  * (C) 1999-2003 Lars Knoll (knoll@kde.org)
- * Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
+ * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
 
 namespace WebCore {
 
+static bool isValidCSSIdentifier(const String& string)
+{
+    unsigned length = string.length();
+    if (!length)
+        return false;
+
+    const UChar* characters = string.characters();
+    UChar c = characters[0];
+    if (!(c == '_' || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c >= 0x80))
+        return false;
+
+    for (unsigned i = 1; i < length; ++i) {
+        c = characters[i];
+        if (!(c == '_' || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' || c >= 0x80))
+            return false;
+    }
+
+    return true;
+}
+
 // Quotes the string if it needs quoting.
-// We use single quotes for now beause markup.cpp uses double quotes.
-static String quoteStringIfNeeded(const String &string)
+// We use single quotes because serialization code uses double quotes, and it's nice to
+// avoid having to turn all the quote marks into &quot; as we would have to.
+static String quoteStringIfNeeded(const String& string)
 {
-    // For now, just do this for strings that start with "#" to fix Korean font names that start with "#".
-    // Post-Tiger, we should isLegalIdentifier instead after working out all the ancillary issues.
-    if (string[0] != '#')
+    if (isValidCSSIdentifier(string))
         return string;
 
-    // FIXME: Also need to transform control characters into \ sequences.
-    String s = string;
-    s.replace('\\', "\\\\");
-    s.replace('\'', "\\'");
-    return "'" + s + "'";
+    // FIXME: Also need to transform control characters (00-1F) into \ sequences.
+    // FIXME: This is inefficient -- should use a Vector<UChar> instead.
+    String quotedString = string;
+    quotedString.replace('\\', "\\\\");
+    quotedString.replace('\'', "\\'");
+    return "'" + quotedString + "'";
 }
 
 FontFamilyValue::FontFamilyValue(const DeprecatedString& string)
index cd349ff3d91e1d396078793db108c77f8e2915a9..c44333fb190531536ae992db01f41d3167f4ddb9 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.  All rights reserved.
+ * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -63,13 +63,13 @@ using namespace HTMLNames;
 
 static inline bool shouldSelfClose(const Node *node);
 
-static DeprecatedString escapeTextForMarkup(const DeprecatedString &in, bool isAttributeValue)
+static DeprecatedString escapeTextForMarkup(const String& in, bool isAttributeValue)
 {
     DeprecatedString s = "";
 
     unsigned len = in.length();
     for (unsigned i = 0; i < len; ++i) {
-        switch (in[i].unicode()) {
+        switch (in[i]) {
             case '&':
                 s += "&amp;";
                 break;
@@ -86,7 +86,7 @@ static DeprecatedString escapeTextForMarkup(const DeprecatedString &in, bool isA
                 }
                 // fall through
             default:
-                s += in[i];
+                s += DeprecatedChar(in[i]);
         }
     }
 
@@ -106,10 +106,10 @@ static String stringValueForRange(const Node *node, const Range *range)
     return str;
 }
 
-static DeprecatedString renderedText(const Node *node, const Range *range)
+static String renderedText(const Node* node, const Range* range)
 {
     if (!node->isTextNode())
-        return DeprecatedString();
+        return String();
 
     ExceptionCode ec;
     const Text* textNode = static_cast<const Text*>(node);
@@ -192,7 +192,7 @@ static String addNamespace(const AtomicString& prefix, const AtomicString& ns, H
     AtomicStringImpl* foundNS = namespaces.get(pre);
     if (foundNS != ns.impl()) {
         namespaces.set(pre, ns.impl());
-        return " xmlns" + (!prefix.isEmpty() ? ":" + prefix : "") + "=\"" + escapeTextForMarkup(ns.deprecatedString(), true) + "\"";
+        return " xmlns" + (!prefix.isEmpty() ? ":" + prefix : "") + "=\"" + escapeTextForMarkup(ns, true) + "\"";
     }
     
     return "";
@@ -212,7 +212,7 @@ static DeprecatedString startMarkup(const Node *node, const Range *range, EAnnot
                     return stringValueForRange(node, range).deprecatedString();
             }
             bool useRenderedText = annotate && !enclosingNodeWithTag(const_cast<Node*>(node), selectTag);
-            DeprecatedString markup = useRenderedText ? escapeTextForMarkup(renderedText(node, range), false) : escapeTextForMarkup(stringValueForRange(node, range).deprecatedString(), false);
+            DeprecatedString markup = escapeTextForMarkup(useRenderedText ? renderedText(node, range) : stringValueForRange(node, range), false);
             return annotate ? convertHTMLTextToInterchangeFormat(markup, static_cast<const Text*>(node)) : markup;
         }
         case Node::COMMENT_NODE:
@@ -245,14 +245,11 @@ static DeprecatedString startMarkup(const Node *node, const Range *range, EAnnot
                 // We'll handle the style attribute separately, below.
                 if (attr->name() == styleAttr && el->isHTMLElement() && (annotate || convertBlocksToInlines))
                     continue;
-                String value = attr->value();
-                // FIXME: Handle case where value has illegal characters in it, like "
                 if (documentIsHTML)
                     markup += " " + attr->name().localName().deprecatedString();
                 else
                     markup += " " + attr->name().toString().deprecatedString();
-                markup += "=\"" + escapeTextForMarkup(value.deprecatedString(), true) + "\"";
-
+                markup += "=\"" + escapeTextForMarkup(attr->value(), true) + "\"";
                 if (!documentIsHTML && namespaces && shouldAddNamespaceAttr(attr, *namespaces))
                     markup += addNamespace(attr->prefix(), attr->namespaceURI(), *namespaces).deprecatedString();
             }
@@ -267,7 +264,7 @@ static DeprecatedString startMarkup(const Node *node, const Range *range, EAnnot
                 if (convertBlocksToInlines)
                     style->setProperty(CSS_PROP_DISPLAY, CSS_VAL_INLINE, true);
                 if (style->length() > 0)
-                    markup += " " +  styleAttr.localName().deprecatedString() + "=\"" + style->cssText().deprecatedString() + "\"";
+                    markup += " style=\"" + escapeTextForMarkup(style->cssText(), true) + "\"";
             }
             
             if (shouldSelfClose(el)) {
@@ -562,7 +559,7 @@ DeprecatedString createMarkup(const Range *range, Vector<Node*>* nodes, EAnnotat
                     style->setProperty(CSS_PROP_BACKGROUND_IMAGE, "url('" + static_cast<Element*>(fullySelectedRoot)->getAttribute(backgroundAttr) + "')");
                 
                 if (style->length()) {
-                    markups.prepend("<div style='" + style->cssText().deprecatedString() + "'>");
+                    markups.prepend("<div style=\"" + escapeTextForMarkup(style->cssText(), true) + "\">");
                     markups.append("</div>");
                 }
             } else {
@@ -589,8 +586,7 @@ DeprecatedString createMarkup(const Range *range, Vector<Node*>* nodes, EAnnotat
         removeEnclosingMailBlockquoteStyle(style.get(), parentOfLastClosed);
         
         if (style->length() > 0) {
-            // FIXME: Handle case where style->cssText() has illegal characters in it, like "
-            DeprecatedString openTag = DeprecatedString("<span class=\"") + AppleStyleSpanClass + "\" style=\"" + style->cssText().deprecatedString() + "\">";
+            DeprecatedString openTag = DeprecatedString("<span class=\"") + AppleStyleSpanClass + "\" style=\"" + escapeTextForMarkup(style->cssText(), true) + "\">";
             markups.prepend(openTag);
             markups.append("</span>");
         }