Validate position.parent's output

[Reinmar]

Positions and the tree to which they point are independent. It is possible to create and work with positions which point to non-existent places. This is mainly required by OT which works on a virtual states.

However, there's a set of position's properties/methods which do access the model structure. This is mainly done via reading position.parent (which is a getter) which, if the position is incorrect will either crash or return a text node. The first scenario is fairly easy to track and understand. The latter is harder because it fails later, when a text node reaches a code which expects an element.

Therefore, I propose to add validation for positions:

• for position.parent that the returned object is really an element and not a text node,
• (optionally) for all incorrect cases in position.parent – e.g. when you have offsets out of boundaries
• (optionally) for other position's props/methods which don't use position.parent and may throw anyway.

cc @scofalik @pjasiun

[Reinmar]

I'll start with the first option to check what will actually happen with our tests.

[scofalik]

I'd go with all three. If they throw, they throw right now, so additional throwing should not change anything (except for the case when we have a silent error with a position in a text node but it worked for some reason... I wouldn't expect a lot of that, though).

[Reinmar]

OK, it looks very promising:

SUMMARY:
✔ 11068 tests completed
ℹ 21 tests skipped
✖ 3 tests failed

FAILED TESTS:
  MergeOperation
    _validate()
      ✖ should throw an error if source position is invalid
        Chrome 75.0.3770 (Mac OS X 10.14.4)
      AssertionError: expected [Error: FOO] to be an instance of CKEditorError

      + expected - actual


    at assertCKEditorError (webpack:///./packages/ckeditor5-utils/tests/_utils/utils.js?:137:22)
    at expectToThrowCKEditorError (webpack:///./packages/ckeditor5-utils/tests/_utils/utils.js?:115:3)
    at Context.eval (webpack:///./packages/ckeditor5-engine/tests/model/operation/mergeoperation.js?:130:114)


      ✖ should throw an error if target position is invalid
        Chrome 75.0.3770 (Mac OS X 10.14.4)
      AssertionError: expected [Error: FOO] to be an instance of CKEditorError

      + expected - actual


    at assertCKEditorError (webpack:///./packages/ckeditor5-utils/tests/_utils/utils.js?:137:22)
    at expectToThrowCKEditorError (webpack:///./packages/ckeditor5-utils/tests/_utils/utils.js?:115:3)
    at Context.eval (webpack:///./packages/ckeditor5-engine/tests/model/operation/mergeoperation.js?:164:114)


  MoveOperation
    _validate()
      ✖ should throw an error if target or source parent-element specified by position does not exist
        Chrome 75.0.3770 (Mac OS X 10.14.4)
      AssertionError: expected [Error: FOO] to be an instance of CKEditorError

      + expected - actual


    at assertCKEditorError (webpack:///./packages/ckeditor5-utils/tests/_utils/utils.js?:137:22)
    at expectToThrowCKEditorError (webpack:///./packages/ckeditor5-utils/tests/_utils/utils.js?:115:3)
    at Context.eval (webpack:///./packages/ckeditor5-engine/tests/model/operation/moveoperation.js?:181:114)