Remove selectionStart polyfill?

[gaearon]

Looking at this code:

react/packages/react-dom/src/client/ReactInputSelection.js

Lines 91 to 100 in bee4baf

	if ('selectionStart' in input) {
	// Modern browser with input or textarea.
	selection = {
	start: input.selectionStart,
	end: input.selectionEnd,
	};
	} else {
	// Content editable or old IE textarea.
	selection = ReactDOMSelection.getOffsets(input);
	}

When would selectionStart not exist? According to http://help.dottoro.com/ljtfkhio.php, only in IE8. (It might be wrong though.)

It would be nice to check, and remove the relevant code if it's supported everywhere else.

[gaearon]

Tagging as good first issue—the outcome is a report on which browsers support the feature.

[jquense]

I am fairly sure it's ie8 only for inputs/text areas. We just removed the equivalent code in a bunch of libraries with no (observable) issues so far. I'd imagine tho the story is different contenteditable? Maybe the draftjs folks have some insight?

[gaearon]

Tagging @flarnie

[gaearon]

(I haven’t thought about CE but you might be right!)

[Miklet]

I also checked at caniuse. Seems support starts from IE9.

[bs91]

MDN also reports IE9 and up support.

[flarnie]

The comment // Content editable or old IE textarea. is a concern because we rely on contenteditable in Draft.js. Let me look into it a bit.

[flarnie]

This example shows that unlike other inputs or a textarea, 'selectionStart' is not available on a contenteditable div. So let's keep this polyfill, and also add a test for this somewhere. I would love to have a Draft.js fixture, but maybe just a contenteditable fixture or test.
https://codepen.io/anon/pen/ZagBrx?editors=1111
I was just playing with that in latest Firefox.

[flarnie]

For anyone interested in more info:
Only input and textarea are defined as having the text field selection API.
https://www.w3.org/TR/2008/WD-html5-20080610/editing.html#textFieldSelection

In Draft we rely on the similar but different Selection interface -
https://www.w3.org/TR/2008/WD-html5-20080610/editing.html#selection1
which we wrap in an immutable record -
https://github.com/facebook/draft-js/blob/master/src/model/immutable/SelectionState.js

[bs91]

@flarnie Would this test suffice? Mainly checking selection start is working where we expect and showing that it doesn't work in the ce case? That way when it's supported we are aware and can remove the backwards support? Or did you have something else in mind?

[flarnie]

@xxblakefailxx Glad to see a test for this - I should have been more clear that I recommend keeping this polyfill in place, and so the test would verify that it does work for contenteditable. If we reverse the expectation of the last test in the suite you wrote then it looks good to me.

To remove this polyfill would be a breaking change, because that would remove our support for selection in contenteditable, and some text editing libraries may be relying on it.

It looks like Draft relies entirely on the Selection api, which is different, but I imagine that adding this support was based in part on potential use cases with Draft.js. @sophiebits can confirm whether removing this polyfill would also break Draft.js.

[sophiebits]

I know contenteditable inputs don't work properly without this. Maybe there is something we can do in user space (such as in draft itself) but it can't just be removed.

[bs91]

@flarnie @sophiebits Do you know of a way we can manual test that shows the contenteditables not working without the polyfill?

[sophiebits]

Not sure but I think if you comment out this code and try to use any Draft editor then you will probably see the problem. If it's not obvious when typing directly then try cutting/pasting and more complex cursor movements.

[vikramcse]

Is someone working on this issue? If not then I would love to work on this issue :)

[flarnie]

Discussing in the comments we determined that contenteditable containers need this polyfill, and it should not be removed after all at this time. I'm closing this issue, but if there are further action items we can open a new issue.

[sophiebits]

The issue may be specific to Draft. If someone wants to investigate and determine exactly what the problem is, maybe we can remove this code from React.

[flarnie]

It's an issue with any contenteditable div, so anyone using that type of input would need this polyfill, not just Draft.js. My example above is just a div with contenteditable. https://codepen.io/anon/pen/ZagBrx?editors=1111

If we really want to remove it, to avoid a 'gotcha' for users we would want a dev-only warning when they use contenteditable, and we would want to provide examples of how to use the polyfill outside of React.

[sophiebits]

Yes, selectionStart does not exist, but we only need it in order to restore selection, which we may not need for Draft. If we moved that logic into Draft then we could skip the selection restoration logic for contenteditable.

We already have a warning against contenteditable saying it's unsupported.