Skip to content

wip: add support for C14N #119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+770 −33
Draft

wip: add support for C14N #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions binding/exported-functions.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ _xmlAddChild
_xmlAddNextSibling
_xmlAddPrevSibling
_xmlCleanupInputCallbacks
_xmlCopyNode
_xmlCtxtParseDtd
_xmlCtxtReadMemory
_xmlCtxtSetErrorHandler
_xmlCtxtValidateDtd
_xmlDocGetRootElement
_xmlDocSetRootElement
_xmlFree
_xmlFreeDoc
_xmlFreeDtd
_xmlFreeNode
Expand Down Expand Up @@ -74,3 +76,10 @@ _xmlXPathFreeObject
_xmlXPathNewContext
_xmlXPathRegisterNs
_xmlXPathSetContextNode
_xmlC14NDocDumpMemory
_xmlC14NExecute
Comment on lines +79 to +80
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xmlC14NExecute is the superset of all the C14N apis, we shouldn't need to export other functions:

xmlC14NDocSaveTo is NodeSet specialized version of xmlC14NExecute
xmlC14NDocSave and xmlC14NDocDumpMemory are destination specialized version of xmlC14NDocSaveTo

So on top of C implemented xmlC14NExecute, we could implement our own in javascript: a Set<XmlNode>-based visible callback etc

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but at the same time xmlC14NExecute requires a callback and my assumption was that doing a callback for each Node to a JS function would be much slower.
That is why xmlC14NDocDumpMemory was my first choice. That function required no other imports and very minimal code to make it work.
Additionally since not all node types are supported in createNode, I could not figure out how to even implement this callback proper.y

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't worry about the performance. "Premature optimization is the root of all evil". There may be many factors affecting whether it is a concern:

  • Callback from wasm to javascript may not be that slow
  • Callback may not be a significant portion of entire C14NDump (i.e. other parts spend more time)
  • C14NDump may be only a small portion of entire execution
  • It may not be very often to dump only some nodes instead of all nodes (passing NULL for all nodes)

Hmm, comparing node is a problem. Besides the problem of unsupported types of node, createNode will create a new wrapper and directly comparing two wrappers doesn't make any sense.

_xmlBufferCreate
_xmlOutputBufferCreateBuffer
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use xmlOutputBufferCreateIO instead, since we are already using XmlOutputBufferHandler as well as XmlStringOutputBufferHandler.

Refer to XmlDocument.save

_xmlBufferContent
_xmlOutputBufferClose
_xmlBufferFree
1 change: 1 addition & 0 deletions binding/exported-runtime-functions.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
UTF8ToString
addFunction
getValue
setValue
lengthBytesUTF8
stringToUTF8
HEAP32
Expand Down
Loading
Loading