Bug 16693 – Acid3 expects WebKit to raise exceptions for invalid qualified names (affects Acid3 test 25)

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Eric Seidel (no email)]

Acid3 expects WebKit to raise exceptions for invalid qualified names

At least qualified names passed to:
document.implementation.createDocumentType

The bug is in DOMImplementation.cpp:

// FIXME: An implementation of this is still waiting for me to understand the distinction between
// a "malformed" qualified name and one with bad characters in it. For example, is a second colon
// an illegal character or a malformed qualified name? This will determine both what parameters
// this function needs to take and exactly what it will do. Should also be exported so that
// Element can use it too.
static bool qualifiedNameIsMalformed(const String&)
{
    return false;
}

The qualified name parsing rules are actually pretty easy:

http://www.w3.org/TR/REC-xml-names/#ns-qualnames

I think we can fix this to make it spec-compliant w/o raising any compatibility concerns.

[Eric Seidel (no email)]

Created attachment 18230 [details]
Test case for createDocumentType

 .../dom/DOMImplementation/createDocumentType.html  |   13 +++++++++++++
 .../dom/DOMImplementation/resources/TEMPLATE.html  |   13 +++++++++++++
 .../resources/createDocumentType.js                |   14 ++++++++++++++
 3 files changed, 40 insertions(+), 0 deletions(-)

[Eric Seidel (no email)]

A more complicated test case could be created. But the one I attached at least shows the bug.  We should probably actually check the type of exception thrown, but I didn't see a good should* function to check that.

Firefox passes all of the subtests in this test case.

[Eric Seidel (no email)]

*** Bug 17074 has been marked as a duplicate of this bug. ***

[Rob Buis]

Created attachment 19020 [details]
first attempt

This patch incorporates and passes Erics testcase as well as the acid3 test.
Note that in most cases the check could be done inside parseQualifiedName too, though createAttributeNS is a bit different here. To pass the tests this suffices though, and there are no regressions in the current layout tests.
Cheers,

Rob.

[Eric Seidel (no email)]

Comment on attachment 19020 [details]
first attempt

Looks like you have tabs in Document.cpp

Document:: functions don't need to use Document:: to reference static methods in the same class.

Since you're changing method behavior in Element, you will need to add tests for the changed behavior.  (Similar to what was added by bug 16833.)  One probably which will probably stop this bug as well is that createElement calls createElementNS (caused the regression that caused bug 16833 to get rolled out).  Likewise setAttribute might be calling setAtributeNS.  That means that this patch would not only need to test createElementNS and setAttributeNS, but also createElement and setAttribute (it's the lack of testing of those two which is currently blocking bug 16833 from completion).

r- w/o more test cases to prove this is correct.

[Eric Seidel (no email)]

Comment on attachment 19020 [details]
first attempt

(Thank you for the patch btw.)  It's always great to see you pop out of the woodwork and throw up a patch here and there. :)

[Eric Seidel (no email)]

Created attachment 19964 [details]
Unify handling of NAMESPACE_ERR and fix Acid3 test 25

 .../createAttributeNS-namespace-err-expected.txt   |    8 +-
 .../createElementNS-namespace-err-expected.txt     |    8 +-
 WebCore/dom/DOMImplementation.cpp                  |   77 ++++----------------
 WebCore/dom/DOMImplementation.idl                  |    6 +-
 WebCore/dom/Document.cpp                           |   51 ++++++++------
 WebCore/dom/Document.h                             |    9 ++-
 WebCore/dom/Element.cpp                            |    8 +-
 WebCore/editing/FormatBlockCommand.cpp             |    9 ++-
 8 files changed, 72 insertions(+), 104 deletions(-)

[Maciej Stachowiak]

Comment on attachment 19964 [details]
Unify handling of NAMESPACE_ERR and fix Acid3 test 25

r=me

[Eric Seidel (no email)]

r31231

[Eric Seidel (no email)]

r31231

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.