Skip to content

Commit 92ec241

Browse files
eseideleseidel
committed
Bug #: 5404
Submitted by: eseidel Reviewed by: mjs & darin Various fixes to createMarkup and toString code to properly support serialization of XML. http://bugzilla.opendarwin.org/show_bug.cgi?id=5404 * khtml/editing/markup.cpp: (khtml::startMarkup): (khtml::doesHTMLForbidEndTag): (khtml::shouldSelfClose): (khtml::endMarkup): (khtml::markup): (khtml::createFragmentFromMarkup): (khtml::createMarkup): * khtml/html/html_elementimpl.cpp: (HTMLElementImpl::createContextualFragment): (HTMLElementImpl::toString): * khtml/html/htmltokenizer.cpp: (khtml::parseHTMLDocumentFragment): * khtml/html/htmltokenizer.h: * khtml/xml/dom_xmlimpl.cpp: (DOM::ProcessingInstructionImpl::ProcessingInstructionImpl): (DOM::ProcessingInstructionImpl::~ProcessingInstructionImpl): (DOM::ProcessingInstructionImpl::toString): * khtml/xml/dom_xmlimpl.h: (DOM::ProcessingInstructionImpl::sheet): (DOM::ProcessingInstructionImpl::setStyleSheet): Canonical link: https://commits.webkit.org/9300@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@10978 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent c2fdcfa commit 92ec241

7 files changed

Lines changed: 114 additions & 84 deletions

File tree

‎WebCore/ChangeLog-2005-12-19‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,33 @@
1+
2005-10-26 Eric Seidel <eseidel@apple.com>
2+
3+
Reviewed by mjs & darin.
4+
5+
Various fixes to createMarkup and toString code to properly
6+
support serialization of XML.
7+
http://bugzilla.opendarwin.org/show_bug.cgi?id=5404
8+
9+
* khtml/editing/markup.cpp:
10+
(khtml::startMarkup):
11+
(khtml::doesHTMLForbidEndTag):
12+
(khtml::shouldSelfClose):
13+
(khtml::endMarkup):
14+
(khtml::markup):
15+
(khtml::createFragmentFromMarkup):
16+
(khtml::createMarkup):
17+
* khtml/html/html_elementimpl.cpp:
18+
(HTMLElementImpl::createContextualFragment):
19+
(HTMLElementImpl::toString):
20+
* khtml/html/htmltokenizer.cpp:
21+
(khtml::parseHTMLDocumentFragment):
22+
* khtml/html/htmltokenizer.h:
23+
* khtml/xml/dom_xmlimpl.cpp:
24+
(DOM::ProcessingInstructionImpl::ProcessingInstructionImpl):
25+
(DOM::ProcessingInstructionImpl::~ProcessingInstructionImpl):
26+
(DOM::ProcessingInstructionImpl::toString):
27+
* khtml/xml/dom_xmlimpl.h:
28+
(DOM::ProcessingInstructionImpl::sheet):
29+
(DOM::ProcessingInstructionImpl::setStyleSheet):
30+
131
2005-10-26 David Hyatt <hyatt@apple.com>
232

333
Don't allow position:relative to apply to table sections. Fixes the crash described in

‎WebCore/khtml/editing/markup.cpp‎

Lines changed: 63 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "editing/visible_units.h"
3535
#include "html/html_elementimpl.h"
3636
#include "xml/dom_position.h"
37+
#include "xml/dom_xmlimpl.h"
3738
#include "xml/dom2_rangeimpl.h"
3839
#include "rendering/render_text.h"
3940
#include "htmlnames.h"
@@ -42,6 +43,7 @@ using namespace DOM::HTMLNames;
4243

4344
using DOM::AttributeImpl;
4445
using DOM::CommentImpl;
46+
using DOM::ProcessingInstructionImpl;
4547
using DOM::CSSComputedStyleDeclarationImpl;
4648
using DOM::CSSMutableStyleDeclarationImpl;
4749
using DOM::DocumentFragmentImpl;
@@ -71,6 +73,9 @@ using DOM::TextImpl;
7173

7274
namespace khtml {
7375

76+
static inline bool doesHTMLForbidEndTag(const NodeImpl *node);
77+
static inline bool shouldSelfClose(const NodeImpl *node);
78+
7479
static QString escapeHTML(const QString &in)
7580
{
7681
QString s = "";
@@ -173,6 +178,7 @@ static QString renderedText(const NodeImpl *node, const RangeImpl *range)
173178

174179
static QString startMarkup(const NodeImpl *node, const RangeImpl *range, EAnnotateForInterchange annotate, CSSMutableStyleDeclarationImpl *defaultStyle)
175180
{
181+
bool documentIsHTML = node->getDocument()->isHTMLDocument();
176182
unsigned short type = node->nodeType();
177183
switch (type) {
178184
case Node::TEXT_NODE: {
@@ -187,18 +193,14 @@ static QString startMarkup(const NodeImpl *node, const RangeImpl *range, EAnnota
187193
if (defaultStyle) {
188194
NodeImpl *element = node->parentNode();
189195
if (element) {
190-
CSSComputedStyleDeclarationImpl *computedStyle = Position(element, 0).computedStyle();
191-
computedStyle->ref();
192-
CSSMutableStyleDeclarationImpl *style = computedStyle->copyInheritableProperties();
193-
computedStyle->deref();
194-
style->ref();
195-
defaultStyle->diff(style);
196+
SharedPtr<CSSComputedStyleDeclarationImpl> computedStyle = Position(element, 0).computedStyle();
197+
SharedPtr<CSSMutableStyleDeclarationImpl> style = computedStyle->copyInheritableProperties();
198+
defaultStyle->diff(style.get());
196199
if (style->length() > 0) {
197200
// FIXME: Handle case where style->cssText() has illegal characters in it, like "
198201
QString openTag = QString("<span class=\"") + AppleStyleSpanClass + "\" style=\"" + style->cssText().qstring() + "\">";
199202
markup = openTag + markup + "</span>";
200203
}
201-
style->deref();
202204
}
203205
}
204206
return annotate ? convertHTMLTextToInterchangeFormat(markup) : markup;
@@ -207,25 +209,23 @@ static QString startMarkup(const NodeImpl *node, const RangeImpl *range, EAnnota
207209
return static_cast<const CommentImpl *>(node)->toString().qstring();
208210
case Node::DOCUMENT_NODE:
209211
return "";
212+
case Node::PROCESSING_INSTRUCTION_NODE:
213+
return static_cast<const ProcessingInstructionImpl *>(node)->toString().qstring();
210214
default: {
211215
QString markup = QChar('<') + node->nodeName().qstring();
212216
if (type == Node::ELEMENT_NODE) {
213217
const ElementImpl *el = static_cast<const ElementImpl *>(node);
214218
DOMString additionalStyle;
215219
if (defaultStyle && el->isHTMLElement()) {
216-
CSSComputedStyleDeclarationImpl *computedStyle = Position(const_cast<ElementImpl *>(el), 0).computedStyle();
217-
computedStyle->ref();
218-
CSSMutableStyleDeclarationImpl *style = computedStyle->copyInheritableProperties();
219-
computedStyle->deref();
220-
style->ref();
221-
defaultStyle->diff(style);
220+
SharedPtr<CSSComputedStyleDeclarationImpl> computedStyle = Position(const_cast<ElementImpl *>(el), 0).computedStyle();
221+
SharedPtr<CSSMutableStyleDeclarationImpl> style = computedStyle->copyInheritableProperties();
222+
defaultStyle->diff(style.get());
222223
if (style->length() > 0) {
223224
CSSMutableStyleDeclarationImpl *inlineStyleDecl = static_cast<const HTMLElementImpl *>(el)->inlineStyleDecl();
224225
if (inlineStyleDecl)
225-
inlineStyleDecl->diff(style);
226+
inlineStyleDecl->diff(style.get());
226227
additionalStyle = style->cssText();
227228
}
228-
style->deref();
229229
}
230230
NamedAttrMapImpl *attrs = el->attributes();
231231
unsigned length = attrs->length();
@@ -240,25 +240,56 @@ static QString startMarkup(const NodeImpl *node, const RangeImpl *range, EAnnota
240240
if (attr->name() == styleAttr && additionalStyle.length() > 0)
241241
value += "; " + additionalStyle;
242242
// FIXME: Handle case where value has illegal characters in it, like "
243-
// FIXME: Namespaces! XML! Ack!
244-
markup += " " + attr->name().localName().qstring() + "=\"" + value.qstring() + "\"";
243+
if (documentIsHTML)
244+
markup += " " + attr->name().localName().qstring();
245+
else
246+
markup += " " + attr->name().toString().qstring();
247+
markup += "=\"" + value.qstring() + "\"";
245248
}
246249
}
247250
}
248-
markup += node->isHTMLElement() ? ">" : "/>";
251+
252+
if (shouldSelfClose(node)) {
253+
if (node->isHTMLElement())
254+
markup += " "; // XHTML 1.0 <-> HTML compatibility.
255+
markup += "/>";
256+
} else
257+
markup += ">";
258+
249259
return markup;
250260
}
251261
}
252262
}
253263

254-
static QString endMarkup(const NodeImpl *node)
264+
static inline bool doesHTMLForbidEndTag(const NodeImpl *node)
255265
{
256-
bool hasEndTag = node->isElementNode();
257266
if (node->isHTMLElement()) {
258267
const HTMLElementImpl* htmlElt = static_cast<const HTMLElementImpl*>(node);
259-
hasEndTag = (htmlElt->endTagRequirement() != TagStatusForbidden);
268+
return (htmlElt->endTagRequirement() == TagStatusForbidden);
260269
}
261-
if (hasEndTag)
270+
return false;
271+
}
272+
273+
// Rules of self-closure
274+
// 1. all html elements in html documents close with >
275+
// 2. all elements w/ children close with >
276+
// 3. all non-html elements w/o children close with />
277+
// 4. all html elements with a FORBIDDEN close tag, self close in XML docs
278+
static inline bool shouldSelfClose(const NodeImpl *node)
279+
{
280+
bool htmlForbidsEndTag = doesHTMLForbidEndTag(node);
281+
bool documentIsHTML = node->getDocument()->isHTMLDocument();
282+
283+
if (node->isHTMLElement() && (documentIsHTML || !htmlForbidsEndTag))
284+
return false;
285+
else if (!node->hasChildNodes() || htmlForbidsEndTag)
286+
return true;
287+
return false;
288+
}
289+
290+
static QString endMarkup(const NodeImpl *node)
291+
{
292+
if (node->isElementNode() && !shouldSelfClose(node) && !doesHTMLForbidEndTag(node))
262293
return "</" + node->nodeName().qstring() + ">";
263294
return "";
264295
}
@@ -270,27 +301,18 @@ static QString markup(const NodeImpl *startNode, bool onlyIncludeChildren, bool
270301
QString me = "";
271302
for (const NodeImpl *current = startNode; current != NULL; current = includeSiblings ? current->nextSibling() : NULL) {
272303
if (!onlyIncludeChildren) {
273-
if (nodes) {
304+
if (nodes)
274305
nodes->append(current);
275-
}
276306
me += startMarkup(current, 0, DoNotAnnotateForInterchange, 0);
277307
}
278-
279-
bool container = true;
280-
if (current->isHTMLElement()) {
281-
const HTMLElementImpl* h = static_cast<const HTMLElementImpl*>(current);
282-
container = h->endTagRequirement() != TagStatusForbidden;
283-
}
284-
if (container) {
285-
// print children
286-
if (NodeImpl *n = current->firstChild()) {
308+
// print children
309+
if (NodeImpl *n = current->firstChild())
310+
if (!(n->getDocument()->isHTMLDocument() && doesHTMLForbidEndTag(current)))
287311
me += markup(n, false, true, nodes);
288-
}
289-
// Print my ending tag
290-
if (!onlyIncludeChildren) {
291-
me += endMarkup(current);
292-
}
293-
}
312+
313+
// Print my ending tag
314+
if (!onlyIncludeChildren)
315+
me += endMarkup(current);
294316
}
295317
return me;
296318
}
@@ -477,6 +499,7 @@ QString createMarkup(const RangeImpl *range, QPtrList<NodeImpl> *nodes, EAnnotat
477499

478500
DocumentFragmentImpl *createFragmentFromMarkup(DocumentImpl *document, const QString &markup, const QString &baseURL)
479501
{
502+
ASSERT(document->documentElement()->isHTMLElement());
480503
// FIXME: What if the document element is not an HTML element?
481504
HTMLElementImpl *element = static_cast<HTMLElementImpl *>(document->documentElement());
482505

@@ -493,14 +516,7 @@ QString createMarkup(const DOM::NodeImpl *node, EChildrenOnly includeChildren,
493516
QPtrList<DOM::NodeImpl> *nodes, EAnnotateForInterchange annotate)
494517
{
495518
ASSERT(annotate == DoNotAnnotateForInterchange); // annotation not yet implemented for this code path
496-
497-
// FIXME: We could take out this if statement if we had more time to test.
498-
// I'm concerned that making this crash when the document is nil might be too risky a change at the moment.
499-
DocumentImpl *doc = node->getDocument();
500-
assert(doc);
501-
if (doc)
502-
doc->updateLayout();
503-
519+
node->getDocument()->updateLayout();
504520
return markup(node, includeChildren, false, nodes);
505521
}
506522

‎WebCore/khtml/html/html_elementimpl.cpp‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -286,23 +286,15 @@ DocumentFragmentImpl *HTMLElementImpl::createContextualFragment(const DOMString
286286
DocumentFragmentImpl *fragment = new DocumentFragmentImpl(docPtr());
287287
fragment->ref();
288288

289-
if (!getDocument()->isHTMLDocument()) {
290-
bool ret = parseXMLDocumentFragment(html, fragment, this);
291-
292-
if (!ret) {
289+
if (getDocument()->isHTMLDocument())
290+
parseHTMLDocumentFragment(html, fragment);
291+
else {
292+
if (!parseXMLDocumentFragment(html, fragment, this)) {
293293
// FIXME: We should propagate a syntax error exception out here.
294294
fragment->deref();
295295
return 0;
296296
}
297297
}
298-
else
299-
{
300-
HTMLTokenizer tok(docPtr(), fragment);
301-
tok.setForceSynchronous(true); // disable asynchronous parsing
302-
tok.write( html.qstring(), true );
303-
tok.finish();
304-
assert(!tok.processingData()); // make sure we're done (see 3963151)
305-
}
306298

307299
// Exceptions are ignored because none ought to happen here.
308300
int ignoredExceptionCode;
@@ -603,7 +595,7 @@ void HTMLElementImpl::accessKeyAction(bool sendToAnyElement)
603595

604596
DOMString HTMLElementImpl::toString() const
605597
{
606-
if (!hasChildNodes()) {
598+
if (!hasChildNodes() && getDocument()->isHTMLDocument()) {
607599
DOMString result = openTagStartToString();
608600
result += ">";
609601

‎WebCore/khtml/html/htmltokenizer.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1836,4 +1836,13 @@ void HTMLTokenizer::setOnHold(bool _onHold)
18361836
onHold = _onHold;
18371837
}
18381838

1839+
void parseHTMLDocumentFragment(const DOM::DOMString &source, DOM::DocumentFragmentImpl *fragment)
1840+
{
1841+
HTMLTokenizer tok(fragment->docPtr(), fragment);
1842+
tok.setForceSynchronous(true);
1843+
tok.write(source.qstring(), true);
1844+
tok.finish();
1845+
assert(!tok.processingData()); // make sure we're done (see 3963151)
1846+
}
1847+
18391848
}

‎WebCore/khtml/html/htmltokenizer.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,8 @@ class HTMLTokenizer : public Tokenizer, public CachedObjectClient
381381
bool inWrite;
382382
};
383383

384+
void parseHTMLDocumentFragment(const DOM::DOMString &, DOM::DocumentFragmentImpl *);
385+
384386
}
385387

386388
#if APPLE_CHANGES

‎WebCore/khtml/xml/dom_xmlimpl.cpp‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,6 @@ ProcessingInstructionImpl::ProcessingInstructionImpl(DocumentPtr *doc) : Contain
303303
m_target = 0;
304304
m_data = 0;
305305
m_localHref = 0;
306-
m_sheet = 0;
307306
m_cachedSheet = 0;
308307
m_loading = false;
309308
#ifdef KHTML_XSLT
@@ -319,7 +318,6 @@ ProcessingInstructionImpl::ProcessingInstructionImpl(DocumentPtr *doc, DOMString
319318
m_data = _data.impl();
320319
if (m_data)
321320
m_data->ref();
322-
m_sheet = 0;
323321
m_cachedSheet = 0;
324322
m_localHref = 0;
325323
#ifdef KHTML_XSLT
@@ -335,8 +333,6 @@ ProcessingInstructionImpl::~ProcessingInstructionImpl()
335333
m_data->deref();
336334
if (m_cachedSheet)
337335
m_cachedSheet->deref(this);
338-
if (m_sheet)
339-
m_sheet->deref();
340336
}
341337

342338
DOMString ProcessingInstructionImpl::target() const
@@ -492,11 +488,6 @@ bool ProcessingInstructionImpl::checkStyleSheet()
492488
return true;
493489
}
494490

495-
StyleSheetImpl* ProcessingInstructionImpl::sheet() const
496-
{
497-
return m_sheet;
498-
}
499-
500491
bool ProcessingInstructionImpl::isLoading() const
501492
{
502493
if (m_loading)
@@ -535,22 +526,13 @@ void ProcessingInstructionImpl::setStyleSheet(const DOMString &url, const DOMStr
535526
getDocument()->stylesheetLoaded();
536527
}
537528

538-
void ProcessingInstructionImpl::setStyleSheet(CSSStyleSheetImpl* sheet)
539-
{
540-
if (m_sheet)
541-
m_sheet->deref();
542-
m_sheet = sheet;
543-
if (m_sheet)
544-
m_sheet->ref();
545-
}
546-
547529
DOMString ProcessingInstructionImpl::toString() const
548530
{
549531
DOMString result = "<?";
550532
result += m_target;
551533
result += " ";
552534
result += m_data;
553-
result += ">";
535+
result += "?>";
554536
return result;
555537
}
556538

0 commit comments

Comments
 (0)