Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

GH-172 - SetDtd on XmlDocument

This should allow a user to set the doctype on a document. Internally
uses the createIntSubset function and applies the dtd to the document
passed into the function.

There were some issues relating to removal of the dtd by calling
xmlCreateIntSubset so instead we first have to get the dtd node, and
then basically remove it and free it before we can apply the new dtd.
This might be better as a few split api's rather than just one lump api
but I figured it would be a little easier this way.

Signed-off-by: Nick Campbell <nicholas.j.campbell@gmail.com>
  • Loading branch information...
commit 976c04610cee05d0e858769eb20f48c3bb4b9bf1 1 parent 51e63d6
@ncb000gt ncb000gt authored
Showing with 65 additions and 0 deletions.
  1. +40 −0 src/xml_document.cc
  2. +1 −0  src/xml_document.h
  3. +24 −0 test/document.js
View
40 src/xml_document.cc
@@ -87,6 +87,42 @@ XmlDocument::Root(const v8::Arguments& args)
}
v8::Handle<v8::Value>
+XmlDocument::SetDtd(const v8::Arguments& args)
+{
+ v8::HandleScope scope;
+
+ XmlDocument* document = ObjectWrap::Unwrap<XmlDocument>(args.Holder());
+ assert(document);
+
+ v8::String::Utf8Value name_(args[0]->ToString());
+ v8::String::Utf8Value externalId_(args[1]->ToString());
+ v8::String::Utf8Value systemId_(args[2]->ToString());
+
+ //No good way of unsetting the doctype if it is previously set...this allows us to.
+ xmlDtdPtr dtd = xmlGetIntSubset(document->xml_obj);
+
+ xmlUnlinkNode((xmlNodePtr) dtd);
+ xmlFreeNode((xmlNodePtr) dtd);
+
+ if (name_.length() == 0 || !args[0]->IsString())
@defunctzombie Collaborator

The checks can be done by the javascript later leaving the c++ later to be simpler.

@ncb000gt Collaborator
ncb000gt added a note

Troof. Good call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ return ThrowException(v8::Exception::Error(
+ v8::String::New("Name must be filled out, and must be a String")));
+ char* name = strdup(*name_);
@ncb000gt Collaborator
ncb000gt added a note

I included cstring in a later commit.

@defunctzombie Collaborator

Why do you need to duplicate the string only to free it later just a few lines below?

@ncb000gt Collaborator
ncb000gt added a note

I don't recall off the top of my head, was a while ago. I'll re-eval and take it ouf it not needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ char* externalId = NULL;
+ if (args.Length() > 1 && args[1]->IsString() && externalId_.length() > 0) externalId = strdup(*externalId_);
+ char* systemId = NULL;
+ if (args.Length() > 2 && args[2]->IsString() && systemId_.length() > 0) systemId = strdup(*systemId_);
@defunctzombie Collaborator

While it is more verbose, I would prefer if we avoided one line ifs and always used { }. I have found it will lead to fewer mistakes down the read.

@ncb000gt Collaborator
ncb000gt added a note

I'll make the style changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ xmlCreateIntSubset(document->xml_obj, (const xmlChar *)name, (const xmlChar *)externalId, (const xmlChar *)systemId);
+
+ if (name != NULL) free(name);
+ if (externalId != NULL) free(externalId);
+ if (systemId != NULL) free(systemId);
+
+ return scope.Close(args.Holder());
+}
+
+v8::Handle<v8::Value>
XmlDocument::ToString(const v8::Arguments& args)
{
v8::HandleScope scope;
@@ -332,6 +368,10 @@ XmlDocument::Initialize(v8::Handle<v8::Object> target)
NODE_SET_PROTOTYPE_METHOD(constructor_template,
"_validate",
XmlDocument::Validate);
+ NODE_SET_PROTOTYPE_METHOD(constructor_template,
+ "setDtd",
+ XmlDocument::SetDtd);
+
NODE_SET_METHOD(target, "fromXml", XmlDocument::FromXml);
NODE_SET_METHOD(target, "fromHtml", XmlDocument::FromHtml);
View
1  src/xml_document.h
@@ -42,6 +42,7 @@ class XmlDocument : public node::ObjectWrap {
static v8::Handle<v8::Value> New(const v8::Arguments& args);
static v8::Handle<v8::Value> FromHtml(const v8::Arguments& args);
static v8::Handle<v8::Value> FromXml(const v8::Arguments& args);
+ static v8::Handle<v8::Value> SetDtd(const v8::Arguments& args);
// document handle methods
static v8::Handle<v8::Value> Root(const v8::Arguments& args);
View
24 test/document.js
@@ -1,5 +1,29 @@
var libxml = require('../index');
+module.exports.setDtd = function(assert) {
+ var doc = libxml.Document();
+ doc.setDtd("html");
+ assert.ok(doc);
+ assert.equal('<?xml version="1.0" encoding="UTF-8"?>\n<!DOCTYPE html>\n', doc.toString());
+ doc.setDtd("html", "bacon", "bacon");
+ assert.ok(doc);
+ assert.equal('<?xml version="1.0" encoding="UTF-8"?>\n<!DOCTYPE html PUBLIC "bacon" "bacon">\n', doc.toString());
+ doc.setDtd("html", null);
+ assert.ok(doc);
+ assert.equal('<?xml version="1.0" encoding="UTF-8"?>\n<!DOCTYPE html>\n', doc.toString());
+ assert.throws(function() {
+ doc.setDtd(5);
+ });
+ assert.ok(doc);
+ assert.equal('<?xml version="1.0" encoding="UTF-8"?>\n', doc.toString());
+ assert.throws(function() {
+ doc.setDtd();
+ });
+ assert.ok(doc);
+ assert.equal('<?xml version="1.0" encoding="UTF-8"?>\n', doc.toString());
+ assert.done();
+};
+
module.exports.blank = function(assert) {
var doc = libxml.Document();
assert.ok(doc);
@ncb000gt
Collaborator

I included cstring in a later commit.

@defunctzombie

Why do you need to duplicate the string only to free it later just a few lines below?

@defunctzombie

While it is more verbose, I would prefer if we avoided one line ifs and always used { }. I have found it will lead to fewer mistakes down the read.

@defunctzombie

The checks can be done by the javascript later leaving the c++ later to be simpler.

@ncb000gt
Collaborator

Troof. Good call.

@ncb000gt
Collaborator

I don't recall off the top of my head, was a while ago. I'll re-eval and take it ouf it not needed.

@ncb000gt
Collaborator

I'll make the style changes.

Please sign in to comment.
Something went wrong with that request. Please try again.