<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" /><style type="text/css"><!--
#msg dl { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fc0 solid; padding: 6px; }
#msg ul, pre { overflow: auto; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<title>[23969] trunk</title>
</head>
<body>
<div id="msg">
<dl>
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/23969">23969</a></dd>
<dt>Author</dt> <dd>darin</dd>
<dt>Date</dt> <dd>2007-07-03 17:04:19 -0700 (Tue, 03 Jul 2007)</dd>
</dl>
<h3>Log Message</h3>
<pre>LayoutTests:
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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsfastinspectorstyleexpectedtxt">trunk/LayoutTests/fast/inspector/style-expected.txt</a></li>
<li><a href="#trunkWebCoreChangeLog">trunk/WebCore/ChangeLog</a></li>
<li><a href="#trunkWebCorecssFontFamilyValuecpp">trunk/WebCore/css/FontFamilyValue.cpp</a></li>
<li><a href="#trunkWebCoreeditingmarkupcpp">trunk/WebCore/editing/markup.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestseditingstylefontfamilywithspaceexpectedtxt">trunk/LayoutTests/editing/style/font-family-with-space-expected.txt</a></li>
<li><a href="#trunkLayoutTestseditingstylefontfamilywithspacehtml">trunk/LayoutTests/editing/style/font-family-with-space.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (23968 => 23969)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2007-07-03 23:56:44 UTC (rev 23968)
+++ trunk/LayoutTests/ChangeLog 2007-07-04 00:04:19 UTC (rev 23969)
</span><span class="lines">@@ -1,3 +1,14 @@
</span><ins>+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.
+
</ins><span class="cx"> 2007-07-03 Adele Peterson <adele@apple.com>
</span><span class="cx">
</span><span class="cx"> Reviewed by Darin.
</span></span></pre></div>
<a id="trunkLayoutTestseditingstylefontfamilywithspaceexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/editing/style/font-family-with-space-expected.txt (0 => 23969)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/editing/style/font-family-with-space-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/style/font-family-with-space-expected.txt 2007-07-04 00:04:19 UTC (rev 23969)
</span><span class="lines">@@ -0,0 +1,16 @@
</span><ins>+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
</ins></span></pre></div>
<a id="trunkLayoutTestseditingstylefontfamilywithspacehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/editing/style/font-family-with-space.html (0 => 23969)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/editing/style/font-family-with-space.html (rev 0)
+++ trunk/LayoutTests/editing/style/font-family-with-space.html 2007-07-04 00:04:19 UTC (rev 23969)
</span><span class="lines">@@ -0,0 +1,11 @@
</span><ins>+<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>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastinspectorstyleexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/inspector/style-expected.txt (23968 => 23969)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/inspector/style-expected.txt 2007-07-03 23:56:44 UTC (rev 23968)
+++ trunk/LayoutTests/fast/inspector/style-expected.txt 2007-07-04 00:04:19 UTC (rev 23969)
</span><span class="lines">@@ -49,6 +49,6 @@
</span><span class="cx"> RenderText {#text} at (0,216) size 75x18
</span><span class="cx"> text run at (0,216) width 75: "color: white"
</span><span class="cx"> RenderBR {BR} at (75,230) size 0x0
</span><del>- 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
</del><ins>+ 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
</ins></span></pre></div>
<a id="trunkWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/ChangeLog (23968 => 23969)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/ChangeLog 2007-07-03 23:56:44 UTC (rev 23968)
+++ trunk/WebCore/ChangeLog 2007-07-04 00:04:19 UTC (rev 23969)
</span><span class="lines">@@ -1,3 +1,24 @@
</span><ins>+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.
+
</ins><span class="cx"> 2007-07-03 Sam Weinig <sam@webkit.org>
</span><span class="cx">
</span><span class="cx"> Reviewed by Darin.
</span></span></pre></div>
<a id="trunkWebCorecssFontFamilyValuecpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/css/FontFamilyValue.cpp (23968 => 23969)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/css/FontFamilyValue.cpp 2007-07-03 23:56:44 UTC (rev 23968)
+++ trunk/WebCore/css/FontFamilyValue.cpp 2007-07-04 00:04:19 UTC (rev 23969)
</span><span class="lines">@@ -1,8 +1,6 @@
</span><span class="cx"> /**
</span><del>- * This file is part of the DOM implementation for KDE.
- *
</del><span class="cx"> * (C) 1999-2003 Lars Knoll (knoll@kde.org)
</span><del>- * Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
</del><ins>+ * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * This library is free software; you can redistribute it and/or
</span><span class="cx"> * modify it under the terms of the GNU Library General Public
</span><span class="lines">@@ -27,20 +25,40 @@
</span><span class="cx">
</span><span class="cx"> namespace WebCore {
</span><span class="cx">
</span><ins>+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;
+}
+
</ins><span class="cx"> // Quotes the string if it needs quoting.
</span><del>-// We use single quotes for now beause markup.cpp uses double quotes.
-static String quoteStringIfNeeded(const String &string)
</del><ins>+// 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)
</ins><span class="cx"> {
</span><del>- // 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] != '#')
</del><ins>+ if (isValidCSSIdentifier(string))
</ins><span class="cx"> return string;
</span><span class="cx">
</span><del>- // FIXME: Also need to transform control characters into \ sequences.
- String s = string;
- s.replace('\\', "\\\\");
- s.replace('\'', "\\'");
- return "'" + s + "'";
</del><ins>+ // 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 + "'";
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> FontFamilyValue::FontFamilyValue(const DeprecatedString& string)
</span></span></pre></div>
<a id="trunkWebCoreeditingmarkupcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/editing/markup.cpp (23968 => 23969)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/editing/markup.cpp 2007-07-03 23:56:44 UTC (rev 23968)
+++ trunk/WebCore/editing/markup.cpp 2007-07-04 00:04:19 UTC (rev 23969)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2004, 2005, 2006 Apple Computer, Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="cx"> * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -63,13 +63,13 @@
</span><span class="cx">
</span><span class="cx"> static inline bool shouldSelfClose(const Node *node);
</span><span class="cx">
</span><del>-static DeprecatedString escapeTextForMarkup(const DeprecatedString &in, bool isAttributeValue)
</del><ins>+static DeprecatedString escapeTextForMarkup(const String& in, bool isAttributeValue)
</ins><span class="cx"> {
</span><span class="cx"> DeprecatedString s = "";
</span><span class="cx">
</span><span class="cx"> unsigned len = in.length();
</span><span class="cx"> for (unsigned i = 0; i < len; ++i) {
</span><del>- switch (in[i].unicode()) {
</del><ins>+ switch (in[i]) {
</ins><span class="cx"> case '&':
</span><span class="cx"> s += "&amp;";
</span><span class="cx"> break;
</span><span class="lines">@@ -86,7 +86,7 @@
</span><span class="cx"> }
</span><span class="cx"> // fall through
</span><span class="cx"> default:
</span><del>- s += in[i];
</del><ins>+ s += DeprecatedChar(in[i]);
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -106,10 +106,10 @@
</span><span class="cx"> return str;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static DeprecatedString renderedText(const Node *node, const Range *range)
</del><ins>+static String renderedText(const Node* node, const Range* range)
</ins><span class="cx"> {
</span><span class="cx"> if (!node->isTextNode())
</span><del>- return DeprecatedString();
</del><ins>+ return String();
</ins><span class="cx">
</span><span class="cx"> ExceptionCode ec;
</span><span class="cx"> const Text* textNode = static_cast<const Text*>(node);
</span><span class="lines">@@ -192,7 +192,7 @@
</span><span class="cx"> AtomicStringImpl* foundNS = namespaces.get(pre);
</span><span class="cx"> if (foundNS != ns.impl()) {
</span><span class="cx"> namespaces.set(pre, ns.impl());
</span><del>- return " xmlns" + (!prefix.isEmpty() ? ":" + prefix : "") + "=\"" + escapeTextForMarkup(ns.deprecatedString(), true) + "\"";
</del><ins>+ return " xmlns" + (!prefix.isEmpty() ? ":" + prefix : "") + "=\"" + escapeTextForMarkup(ns, true) + "\"";
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> return "";
</span><span class="lines">@@ -212,7 +212,7 @@
</span><span class="cx"> return stringValueForRange(node, range).deprecatedString();
</span><span class="cx"> }
</span><span class="cx"> bool useRenderedText = annotate && !enclosingNodeWithTag(const_cast<Node*>(node), selectTag);
</span><del>- DeprecatedString markup = useRenderedText ? escapeTextForMarkup(renderedText(node, range), false) : escapeTextForMarkup(stringValueForRange(node, range).deprecatedString(), false);
</del><ins>+ DeprecatedString markup = escapeTextForMarkup(useRenderedText ? renderedText(node, range) : stringValueForRange(node, range), false);
</ins><span class="cx"> return annotate ? convertHTMLTextToInterchangeFormat(markup, static_cast<const Text*>(node)) : markup;
</span><span class="cx"> }
</span><span class="cx"> case Node::COMMENT_NODE:
</span><span class="lines">@@ -245,14 +245,11 @@
</span><span class="cx"> // We'll handle the style attribute separately, below.
</span><span class="cx"> if (attr->name() == styleAttr && el->isHTMLElement() && (annotate || convertBlocksToInlines))
</span><span class="cx"> continue;
</span><del>- String value = attr->value();
- // FIXME: Handle case where value has illegal characters in it, like "
</del><span class="cx"> if (documentIsHTML)
</span><span class="cx"> markup += " " + attr->name().localName().deprecatedString();
</span><span class="cx"> else
</span><span class="cx"> markup += " " + attr->name().toString().deprecatedString();
</span><del>- markup += "=\"" + escapeTextForMarkup(value.deprecatedString(), true) + "\"";
-
</del><ins>+ markup += "=\"" + escapeTextForMarkup(attr->value(), true) + "\"";
</ins><span class="cx"> if (!documentIsHTML && namespaces && shouldAddNamespaceAttr(attr, *namespaces))
</span><span class="cx"> markup += addNamespace(attr->prefix(), attr->namespaceURI(), *namespaces).deprecatedString();
</span><span class="cx"> }
</span><span class="lines">@@ -267,7 +264,7 @@
</span><span class="cx"> if (convertBlocksToInlines)
</span><span class="cx"> style->setProperty(CSS_PROP_DISPLAY, CSS_VAL_INLINE, true);
</span><span class="cx"> if (style->length() > 0)
</span><del>- markup += " " + styleAttr.localName().deprecatedString() + "=\"" + style->cssText().deprecatedString() + "\"";
</del><ins>+ markup += " style=\"" + escapeTextForMarkup(style->cssText(), true) + "\"";
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if (shouldSelfClose(el)) {
</span><span class="lines">@@ -562,7 +559,7 @@
</span><span class="cx"> style->setProperty(CSS_PROP_BACKGROUND_IMAGE, "url('" + static_cast<Element*>(fullySelectedRoot)->getAttribute(backgroundAttr) + "')");
</span><span class="cx">
</span><span class="cx"> if (style->length()) {
</span><del>- markups.prepend("<div style='" + style->cssText().deprecatedString() + "'>");
</del><ins>+ markups.prepend("<div style=\"" + escapeTextForMarkup(style->cssText(), true) + "\">");
</ins><span class="cx"> markups.append("</div>");
</span><span class="cx"> }
</span><span class="cx"> } else {
</span><span class="lines">@@ -589,8 +586,7 @@
</span><span class="cx"> removeEnclosingMailBlockquoteStyle(style.get(), parentOfLastClosed);
</span><span class="cx">
</span><span class="cx"> if (style->length() > 0) {
</span><del>- // FIXME: Handle case where style->cssText() has illegal characters in it, like "
- DeprecatedString openTag = DeprecatedString("<span class=\"") + AppleStyleSpanClass + "\" style=\"" + style->cssText().deprecatedString() + "\">";
</del><ins>+ DeprecatedString openTag = DeprecatedString("<span class=\"") + AppleStyleSpanClass + "\" style=\"" + escapeTextForMarkup(style->cssText(), true) + "\">";
</ins><span class="cx"> markups.prepend(openTag);
</span><span class="cx"> markups.append("</span>");
</span><span class="cx"> }
</span></span></pre>
</div>
</div>
</body>
</html>