Greetings all, While working on the bug 44288<https://bugs.webkit.org/show_bug.cgi?id=44288>, it came to my attention that the default constructor of QualifiedName is behind a flag QNAME_DEFAULT_CONSTRUCTOR, and I can't find a qualified name that stands for invalid / null value. Is there any designated qualified name object that stands for invalid / null value? Invalid / null qualified name will be useful in the context like: http://trac.webkit.org/browser/trunk/WebCore/editing/markup.cpp?rev=65854#L4... : void MarkupAccumulator::appendText(Vector<UChar>& out, Text* text) { const QualifiedName* parentName = 0; if (text->parentElement()) parentName = &static_cast<Element*>(text->parentElement())->tagQName(); if (parentName && (*parentName == scriptTag || *parentName == styleTag || *parentName == xmpTag)) { appendUCharRange(out, ucharRange(text, m_range)); return; } if (!shouldAnnotate() || (parentName && *parentName == textareaTag)) { appendEscapedContent(out, ucharRange(text, m_range), text->document()->isHTMLDocument()); return; } ... where I obtain the qualified name of the parent element which may or may not exist. If we had a null qualified name, above code can be re-written as: void MarkupAccumulator::appendText(Vector<UChar>& out, Text* text) { QualifiedName parentName = nullName; if (text->parentElement()) parentName = &static_cast<Element*>(text->parentElement())->tagQName(); if (parentName == scriptTag || parentName == styleTag || parentName == xmpTag) { appendUCharRange(out, ucharRange(text, m_range)); return; } if (!shouldAnnotate() || parentName == textareaTag) { appendEscapedContent(out, ucharRange(text, m_range), text->document()->isHTMLDocument()); return; } ... IMHO, new code is easier to read and has very little drawbacks (possible slowdown use to construction of QualifiedName). I could have used htmlTag or any other qualified name different from the four names used above, but that wouldn't be semantically correct. Ryosuke Niwa Software Engineer Google Inc.
While I don’t strongly oppose adding a null value, there are some benefits to not having one. Not having to handle that special case can keep code simpler. And if you don’t have a null value or empty value then you don’t have to answer the question of whether these two are the same thing or not. I don’t think this particular call site cries out for the null value. For example, I think using the QualifiedName* here is fine. And you could also use any qualified name other than scriptTag, styleTag, xmpTag, or textareaTag. For example, you might think it’s logical to use bodyTag or something like that. -- Darin
I think this callsite reads better w/o the QualifiedName*, but Darin is right, that could just be htmlTag or whatever with a comment explaining. This site could also crete its own: static QualifiedName nullName(nullAtom, nullAtom, nullAtom) if it really wanted. -eric On Tue, Aug 24, 2010 at 12:29 PM, Darin Adler <darin@apple.com> wrote:
While I don’t strongly oppose adding a null value, there are some benefits to not having one. Not having to handle that special case can keep code simpler. And if you don’t have a null value or empty value then you don’t have to answer the question of whether these two are the same thing or not.
I don’t think this particular call site cries out for the null value. For example, I think using the QualifiedName* here is fine. And you could also use any qualified name other than scriptTag, styleTag, xmpTag, or textareaTag. For example, you might think it’s logical to use bodyTag or something like that.
-- Darin
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
participants (3)
-
Darin Adler
-
Eric Seidel
-
Ryosuke Niwa