Quicktags: Can't add them using just a keyboard in IE

[Cheffheid]

When using just a keyboard, I seem to be unable to make use of the quick tags.

This seems to be an issue just with Internet Explorer. Firefox and Chrome seem to be functioning as intended.

Steps to reproduce:

1. Find a comment in the admin and toggle "Quick Edit"
2. Select part of the text (this step doesn't necessarily have to be done with the keyboard)
3. Use Shift+Tab to shift focus from the text area to a button (for example, b or i)
4. Hit Enter to trigger the button

In IE, it seems to place the opening tag on either end of the selection. In Chrome and Firefox, it properly wraps the selection in the selected tag.

For illustration, here's some GIFs I've made:

Internet Explorer:

Chrome:

[Cheffheid]

Some additional details to clarify:

So far it seems that it affects IE8 and IE11. IE10 seems to handle it as expected.

The opening tag always gets placed where you started the selection.

[afercia]

Standard compliant browsers and (old?) Internet Explorer have different ways to handle text selection. This is especially true inside a textarea, quoting @azaozz :) ​yep, IE cannot keep a selection in blurred elements

Maybe the simplest way to try to improve here is to get and store the text selection before the textarea is blurred, i.e. when reverse tabbing from the textarea to the quicktags toolbar, see the attached experimental patch.

I can test only in Internet Explorer 8 so for now this is experimental, would need someone else help to test in IE 9+ before moving on and refining the patch. In IE 8 seems to work nicely, see the animated GIF below.

How to test:

• in different IE versions
• use just your keyboard, Tab to navigate and Enter to activate the buttons
• try to add quick-tags in all the possible ways

Additionally, the "Close All" button is now enabled only when there are "pending" open quicktags to close.

[Cheffheid]

As it turns out, IE11 replaced selection with getSelection() and so this patch doesn't really do much for that browser. See also ​https://msdn.microsoft.com/en-us/library/ie/bg182625%28v=vs.85%29.aspx#legacyAPIs

Couldn't get createRange() to work on it either (reading into it more it should work, so I'm not sure what I did wrong just yet), and simply passing whatever getSelection() returns causes only the opening tag to appear in the right place.

I'll fiddle with it some more when I get the chance, I wouldn't doubt that I was looking at it wrong at 5:30am. :)

[afercia]

To my understanding, for IE 8- createRange() is a method of the selection and for IE 9+ and standard compliant browser is a method of the document, see for example what TinyMCE does:

rng = selection.createRange ? selection.createRange() : doc.createRange();

Additionally, quicktags.js is a bit old code and maybe the checks it does should be changed:

if ( document.selection ) { // IE

    `document.selection` is still used by IE 9 and 10 so the condition is true for IE 10-
    but then in this block `createRange()` works only in IE 8- so IE 9 and 10 will fail

} else if ( canvas.selectionStart || canvas.selectionStart === 0 ) { // FF, WebKit, Opera

    condition should be true for IE 11 too

} else { // other browsers?

    no idea.

}

Could someone more familiar than me with text selections please confirm this? Thanks very much.

[afercia]

Seen some documentation and examples, to my understanding:

• document.getSelection() (equivalent of window.getSelection() ) doesn't work in Firefox on a textarea

• selectionStart and selectionEnd properties in IE work starting from IE 9 ​https://msdn.microsoft.com/en-us/library/ie/ff974768(v=vs.85).aspx ​https://msdn.microsoft.com/en-us/library/ie/ff974767(v=vs.85).aspx

I think we should reverse the logic in quicktags and check first for selectionStart and selectionEnd support, then for document.selection (IE 8-)

[Cheffheid]

I think this'll do it. Not quite the same logic as you suggest, but it's similar I guess.

IE11 supports getSelection, but doesn't return anything when the selection is in a textarea. When getSelection is supported, it will instead store the start and end of the selection in an array. If it's not, it'll continue business as usual with a snippet I had found in bookmarklet.js.

Might not need the document.getSelection() check, I'm not too sure.

Anyhow, with this patch it seems that other modern browsers will now set keyboardSelectionIE (because they all support window.getSelection(), so that makes sense) and so I'm not sure if the additional else clause on lines 537 - 540 are even needed anymore either.

I did a quick test with mouse and keyboard in IE 11, Firefox 36, and Chrome 41 on Windows and all seems to be functioning as expected. Would appreciate some help testing and help/direction cleaning/reviewing the code. :)

[afercia]

Replying to Cheffheid:

I think this'll do it. Not quite the same logic as you suggest, but it's similar I guess.

Hi Cheffheid, yup it's similar

with a snippet I had found in bookmarklet.js.

BTW it's a different case because in bookmarklet.js getSelection() is used to get the selection on the "page" while when it comes to input fields and textareas getSelection() might not work. Moreover, to my understanding the selection won't be "remembered" when blurring the textarea, while modern browser should "remember" selectionStart and selectionEnd because they're properties. See you on Slack, would like to discuss a bit this with you :)

[afercia]

Quick and dirty patch just for debugging purposes :) Would love if someone could test this in IE 9, IE 10 and IE 11. I can test in other browsers including IE 8, feel free to do the same.
How to test:

• go in the admin Comments screen
• open your IE dev tools console (press F12), go in the Script tab and watch the console output
• click "Quick Edit" for a comment
• once in the textarea, use just your keyboard to make a selection
• reverse tab from the textarea to the buttons, note the console output
• press a button, e.g. "code" or "bold", note the console output

It's important to note which method is supported but especially which selected text is returned both when moving to the buttons and after you pressed a button.
Hopefully, selectionStart and selectionEnd should always work in IE 9, 10 and 11.

[Cheffheid]

Results for IE11:

It supports all of it, but it does weird things with the selection after reverse tabbing out of the text area.

It's why I ended up storing the selection's start and end in an array when the getSelection() check returns true and the reverse tab out of the textarea happens :).

IE9, properly puts the tags in:

IE10, properly puts the tags in as well:

[afercia]

Thanks very much Jeff :) so let's summarize

All modern browsers support selectionStart/End
IE 11 supports selectionStart/End and window.getSelection
IE 10 and 9 are in a "mixed" situation and support selectionStart/End, window.getSelection and document.selection
IE 8 supports just document.selection

Please consider window.getSelection is not intended for input and textareas, and for sure it doesn't work in Firefox, see ​https://bugzilla.mozilla.org/show_bug.cgi?id=85686

It's why I ended up storing the selection's start and end in an array when the getSelection() check returns true and the reverse tab out of the textarea happens :).

Sure that's what we should do :) but maybe we shouldn't check for getSelection() ? I'd say to reverse the logic, modern browsers first and at this point maybe use the stored values for all browsers:

// on reverse tabbing store proper values for all browsers: maybe we could use just one array
// with values for canvas.selectionStart, canvas.selectionEnd, document.selection.createRange()

if ( canvas.selectionStart || canvas.selectionStart === 0 ) {
    // true for IE 11, IE 10, IE 9 and modern browsers
    check if there are stored startPos and endPos and use them
    or fallback to canvas.selectionStart and canvas.selectionEnd
    ...
} else if ( document.selection ) { // now this is just for IE 8
    use the stored keyboardSelectionIE or fallback (see previous patch) 
    ...
}

Thoughts?

[Cheffheid]

New patch, refreshed against trunk.

I've also flipped checks around in the callback function on line 502, since IE9 and up supports canvas.selectionStart anyway and so this way document.selection is only used for the older browsers that don't.

Similarly, the OnTab function uses canvas.selectionStart first before trying document.selection to store whatever the value needs to be to add the tags.

I've done some quick tests with keyboard and mouse in IE8, IE9, IE10, IE11, Firefox 38, and Chrome 43 and it looks good. As before, need someone to confirm and double check for me. :)

[afercia]

Tested in old browsers like IE 8 and Opera 12.17. Looks good.

[rianrietveld]

We need a JavaScript expert to review this code: @azaozz and @iseulde, if you have time, can you take a look at the latest patch?

[azaozz]

Few "first look" things:

• We only need this for IE, no need to run it in other browsers.
• Don't think we need to disable the "close all tags" button. Was actually thinking we should remove it from the default configuration.
• Needs some optimization.

We would still need to keep both document.selection and 'selectionStart' in canvas methods as IE11 still needs this and doesn't support the old IE ways. I'll look more next week.

[afercia]

Replying to azaozz:

• Don't think we need to disable the "close all tags" button. Was actually thinking we should remove it from the default configuration.

I'd agree. The first time I looked into this, I had troubles to understand what that button exactly does. Discussed a bit in today's accessibility meeting on Slack and looks like I'm not the only one :)

[Cheffheid]

Thanks for the quick look!

I'll make a new patch without the disabling "close all tags" stuff. Won't hear me complain if that button gets removed either. :)

I'm not entirely sure if I get your other comments though. I've mainly swapped checks around and made selectionStart the "main" check before falling back to document.selection since IE8 seems to be the only one that absolutely requires the latter for this to work. Apart from that, it saves the start and end positions of the selection when shift+tab is pressed in the textarea.

Regardless of the method used to get the selection, IE8 and up all seem to have issues retaining the selection when a button is triggered. As much as I'd like to make it IE-specific, I'm not really aware of a way to specifically target IE (short of checking the user agent). selectionStart === selectionEnd could also mean that there is no selection But it is also only true for IE when a button is triggered, not when shift+tab is pressed to get out of the textarea, assuming a selection was made to begin with.

[Cheffheid]

Just the shift+tab stuff.

[Cheffheid]

Ignore patch 4 if you could. In my rush to get it out today I missed the var declaration for keyboardSelection. Shocked it actually kept working without it, but anyhow.

[obenland]

No movement in a week, let's try again in a future release.

[afercia]

Linking here a relevant conversation on Slack: "is it time to remove the Quicktags buttons from the Text editor?"
​https://wordpress.slack.com/archives/core-editor/p1475103109000800

[afercia]

Needs feedback from the editor's team :) If the Quicktags buttons are going to stay, they should work in all browsers. If they're going to be replaced with some new editor, this ticket should be closed.

[audrasjb]

The new editor removed quick tags, even from the Classic Block.

Tagging as close for the moment.

[afercia]

Discussed during today's accessibility bug scrub. Agreed to close, as now the default editor is Gutenberg and these quick-tags buttons still live only in the Classic Editor text mode.