DomConverter throws an error when found <!-- comment -->

[pomek]

This line throws an error: Uncaught TypeError: Cannot read property 'toLowerCase' of undefined when domNode is a Comment.

I figured out it during our new testing environment (Karma & Webpack). Karma inserts a comment to body tag. DomConverter assumes that found domNode is an instance of HTMLElement.

For the first iteration, domNode is a document.body. For the second iteration, domNode is an instance of Comment.

I also reproduced this error in Bender.

Steps to reproduce:

1. Add to this file - tests/view/observer/__template__.html - some HTML comment: <!-- some comment -->
2. Run the test: tests/engine/view/observer/domeventdata

I know this issue is like "edge case" but we should prevent it.

[Reinmar]

Let's not forget to revert this: https://github.com/ckeditor/ckeditor5-engine/pull/662/files#diff-d10955305069dfc2088d6406b09b3447R17

[Reinmar]

What do we actually want to do with this? Do we want to convert comments to view? I guess we'll need to support comments in the view cause otherwise you wouldn't be able to easily produce data with comments.

However, if we'd need to work on converting comments to the model and back... that would be crazy. Of course, we're not going to introduce comments in the model, but one might want to convert comments to some elements or attrs. This would be quite a lot of work.

[Reinmar]

cc @pjasiun @szymonkups could this method return null for comments for now? Or will it break something?

[pjasiun]

I agree with both: we should, at some point, handle comments conversion, to stop trimming all comments (I already see these SO issues: "CKE5 remove my comments!"). But that's also true that it will be some work to be done: view element, model element, converters, both tree walkers improvements and all function which use tree walkers.

For now, returning null should work fine. We need to handle null anyways for empty text nodes which have no view representation, so it should work for comments too.

[scofalik]

Maybe it would be most safe to convert comments to collapsed markers? There are a lot of advantages to it.

[Reinmar]

Maybe it would be most safe to convert comments to collapsed markers? There are a lot of advantages to it.

In the future, this may be some idea. But I'd just ignore comments now.