(WIP) Use only public API for getNodeForCharacterOffset test

[accordeiro]

This PR refers to #11299 - Express more tests via public API.

I’m trying to find a good approach for testing the getNodeForCharacterOffset() using only public API.

I’ve already mapped the call sequence from the public API to this function:

---

• getNodeForCharacterOffset() @ getNodeForCharacterOffset.js
• setOffsets() @ ReactDOMSelection.js
• setSelection() @ ReactInputSelection.js
• restoreSelection() @ ReactInputSelection.js

Explicitly in this if clause (From ReactInputSelection.js):

if (curFocusedElem !== priorFocusedElem && isInDocument(priorFocusedElem)) {
  if (hasSelectionCapabilities(priorFocusedElem)) {
    setSelection(priorFocusedElem, priorSelectionRange);
	}
  ...
}

• DOMRenderer.resetAfterCommit() @ ReactDOM.js

---

The tests I’ve written so far manage to reach setSelection() @ ReactInputSelection.js , but there’s still an if clause in setSelection() before we actually get to getNodeForCharacterOffset(), which is:

  if ('selectionStart' in input) {
    input.selectionStart = start;
    input.selectionEnd = Math.min(end, input.value.length);
  } else {
    ReactDOMSelection.setOffsets(input, offsets);
  }

I’m now testing how we could cause the code to trigger the else clause – any insights on how I can achieve this?

Thanks!

[gaearon]

You can check where selectionStart is defined. For example it may be HTMLInputElement.prototype or HTMLTextAreaElement.prototype (or both of them).

Then you want to delete it before the test, but restore it after regardless of whether the test passed or not. Something like:

const inputDescriptor = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'selectionStart');
const textAreaDescriptor = Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, 'selectionStart');

delete HTMLInputElement.prototype.selectionStart;
delete HTMLTextAreaElement.prototype.selectionStart;
try {
  // your test
} finally {
  Object.defineProperty(HTMLInputElement, 'selectionStart', inputDescriptor);
  Object.defineProperty(HTMLTextAreaElement, 'selectionStart', textAreaDescriptor);
}

[accordeiro]

That makes sense – thanks for the tip!

I got sick last weekend and didn't have the chance to follow up, but I should be able to send an updated PR in the next couple of days – is this ok?

[gaearon]

Sure, get well!

[accordeiro]

Hi, @gaearon –

Sorry for the delay – the last weeks of the year are hectic around here. I've been trying to come up with an approach for writing these tests via public API, but it's a bit harder than I imagined at first (I'm still very motivated to move forward, though 😄)

I've made the changes you've requested, and even managed to trigger actual calls to getNodeForCharacterOffset() using only the public API, but I'm currently facing the following two issues:

1. Since some of the selection functionality isn't implemented in jsdom yet (like window.getSelection() or document.createRange()), I find myself trying to mock some of those functions (like here), but I'm not sure this is the right path.
2. Since the intermediate functions between the public API and getNodeForCharacterOffset also use non-implemented DOM functions, I'm a bit lost on what needs to be mocked and what not.

Any insights on how I can move forward? Thanks for the help and patience :)

[Also, please don't mind the messy code, I was just trying to illustrate my reasoning]

[gaearon]

We've updated jsdom, could you try again?

[accordeiro]

Awesome! I’ll try again, thanks for the update :)

[accordeiro]

Hi, @gaearon –

It still seems like window.getSelection() and document.createRange() haven't been incorporated into jsdom yet (and I haven't seen any suggestion that they'll be added soon).

I can try to figure something out with the jsdom guys, if you believe that's a good idea. Otherwise, do you have any suggestions on how we can alternatively approach this test?

Please let me know your thoughts.

[philipp-spiess]

@accordeiro I've also found myself struggling with the lack of a selection API in JSDom in the past. Is there an upstream issue to track support for that?

As for this PR, I think a bit of mocking might be OK if we:

1. Keep it as simple as possible to avoid confusion.
2. Make sure that we clean up after the test the same way we want to recover from the deleted selectionStart props. This might otherwise create bugs if we try to add another test to the suite.

[accordeiro]

@philipp-spiess there is an upstream issue, but it doesn't look like the DOM selection API implementation is on their roadmap (see jsdom/jsdom#937 for reference).

I'll try to do some mocking keeping in mind the two points you've listed – I should have an update until the end of the week, is that ok?

[philipp-spiess]

@accordeiro No worries, take the time you need 😊 We're glad that you still want to work on this issue.

Your plan sounds great. Keep us updated! And thanks for linking the upstream issue.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[stale]

Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you!