Deal with comment nodes

[Kuznetsov-Ilia]

Why only Element prototype is used when polyfilling dom4 methods? I have library where comment nodes are used. remove/append and other sugar stuff is not available for them. Now i just swap Node and Element
ElementPrototype = (window.Node || window.Element || window.HTMLElement).prototype
I understand that these methods are written just in Element proto in spec, but it is inconvenient for document nodes that does not implement Element prototype...

[WebReflection]

this is not a library, it's a polyfill which aim is to bring standards as meant / documented in browsers that don't support one or more features.

Accordingly, if there are methods that should be on Comments too, I will follow your hint or find a better solution, but if this is not documented or meant like that, you should complain with WHATWG and not with this polyfill.

Which one is this case?

[Kuznetsov-Ilia]

https://dom.spec.whatwg.org/#interface-childnode

[NoInterfaceObject,
 Exposed=Window]
interface ChildNode {
  [Unscopeable] void before((Node or DOMString)... nodes);
  [Unscopeable] void after((Node or DOMString)... nodes);
  [Unscopeable] void replaceWith((Node or DOMString)... nodes);
  [Unscopeable] void remove();
};
DocumentType implements ChildNode;
Element implements ChildNode;
CharacterData implements ChildNode;

You implemented only for Element, but in spec there are also DocumentType and CharacterData

[Kuznetsov-Ilia]

Closest parent interface for all of them is Node.

If you want to live with spec, you must test before/after/replaceWith/remove props in any of DocumentType/Element/CharacterData interfaces and polyfill them in all of them.
https://dom.spec.whatwg.org/#parentnode
prepend/append/query in Document, DocumentFragment, Element

Otherwise document, fragment and comment nodes are abandoned...

[WebReflection]

I don't think anything can possibly happen if I append to a Document ... .right? Or replace it

[Kuznetsov-Ilia]

I think the most effective way is to add all of these methods explicitly to Node prototype without any check. If there will be remove/prepend any other method in Comment, Element, Document, DocumentFragment prototype, the same named method in Node prototype will be overwritten by upper prototype.

[WebReflection]

OK, this might sound like a silly question: if you think all dom4 needed was to just use Node instead of Element or HTMLElement, why didn't you just PR that?

I am all in to make this poly work at its best so don't take my question wrong, I am trying to understand if you are aware of some side effect that might be caused by such change.

Thanks

[Kuznetsov-Ilia]

Sorry, I`m a little bit messy, last commit 85b6232 shows what I mean: we put all methods in Node proto.
Example:
https://developer.mozilla.org/en-US/docs/Web/API/ChildNode/remove#Browser_compatibility
Chrome 23.0 supports remove method for Element and it will use native method, but for CharacterData and DocumentType - will use from Node prototype and etc...

[WebReflection]

OK I am traveling these days but I've given a spin to the logic and read more details about those interfaces.

TL;DR I don't think using Node is a good idea because it will be polluted with unexpected methods.

Moreover, stuff like matches won't be correctly polyfilled because the script will look for prefixed/vendored methods in the wrong prototype.

Accordingly, I think the current way I am moving forward is to fix explicitly before|after|replace|remove in CharacterData, if present, and if DocumentType, still if present, falling back for those methods only to Node in case those constructors are not exposed.

This will de-facto pollute Node with before|after|replace|remove so that CDATA and comments should be also aware of those methods.

Would this work for you?

[WebReflection]

P.S. you can see the test with comment ( very basic one ) in the test page

If you can verify it works with your target browsers I might just flag and push to npm and cdn.

Thank You

[Kuznetsov-Ilia]

I'm new to gh-pages, but l'll try to setup new test at http://qiv.github.io/dom4/test
For me everything is working from Node. I try to understand how it would be proper for this repo to correspond the spec and to be performant.
matches are still looking at Element, but set to Node
I agree with that Node must not be polluted with unexpected methods, but can we expect that some vendor will add to Node interface method like replaceWith? I dont think so - this method will be inaccessible due to upper interface with this name method. We just use javascript native way - prototyping, to polyfill methods that are not implemented. If method is matched at upper prototype, it will not be invoked from Node. It is quite cheap way not to test horrible ua interface implementation.

[WebReflection]

My last change fallbacks to Node but it uses spec-compliant constructors/prototypes to bring in those methods. Does this work?

[Kuznetsov-Ilia]

It seem like everything is fine.
To be honest with spec, it is better not to forget to duplicate prepend/append/query methods to Document and Document Fragment prototypes...