Page MenuHomePhabricator

User can't tab out VisualEditor surfaces, e.g. comment input widgets, media dialog
Closed, ResolvedPublic

Description

When “Visual editing” is enabled and you tab into the editing area, you can't tab out any more.

Example from https://www.mediawiki.org/wiki/Talk:Reading/Web/Projects/Print_Styles

Testing instructions

In addition to the functionality described above (tabbing out of an edit area), we should regression test changing focus on mobile (particularly iOS), so:

  • selecting text
  • opening the toolbar
  • using the link inspector (card)

Pressing Tab or Shift+Tab should leave the editing area in:

  • VisualEditor dialogs, e.g. the caption field in media dialog
  • Flow new topic or reply
  • DiscussionTools new topic or reply

It should not leave the editing area in normal VisualEditor.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Volker_E triaged this task as Medium priority.Aug 7 2017, 2:50 PM
Volker_E updated the task description. (Show Details)

This is true of all VE surfaces, include the TargetWidgets found in media/gallery dialogs:

When editing an image, a media dialog opens to allow editing information about the image (e.g., the caption used). Hitting the tab key allows to move the input focus through some elements, but it gets stuck in the caption rich text field. The input focus is not moving to other elements when hitting the tab key again, making other fields such as "alternative text" not reachable in this way.

This is illustrated below:

Jul-02-2018 13-11-00.gif (340×640 px, 1 MB)

The reason for this is that tab/shift+tab perform actions in some contexts in VE, specifically in a list they change indentation, and in table selections they cycle through cells. It is easy enough to allow change focus to work only in non-list/non-table contexts, but I'm not sure this is the greatest solution.

Esanders renamed this task from User can't tab out focussed “Visual editing” textarea in Flow to User can't tab out VisualEditor surfaces, e.g. Flow input widgets, media dialog.Nov 5 2018, 5:17 PM

Are list/table contexts the only ones affected?

Esanders renamed this task from User can't tab out VisualEditor surfaces, e.g. Flow input widgets, media dialog to User can't tab out VisualEditor surfaces, e.g. comment input widgets, media dialog.May 20 2020, 1:40 PM
Esanders added a project: DiscussionTools.

This also affects the new DiscussionTools reply widget.

Change 612422 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[VisualEditor/VisualEditor@master] ve.ce.Surface: Allow tabbing out of surfaces without indentation features

https://gerrit.wikimedia.org/r/612422

JTannerWMF subscribed.

We will look at this in the fiscal year

The easy way to fix all of this is to unbind tab/shift+tab from VE completely as we already have CTRL+[ and CTRL+] bound (this is what GMail does). The only issue is if this works on other keyboard layouts that may not have easy access to '['.

The only issue is if this works on other keyboard layouts that may not have easy access to '['.

It doesn’t. For example, on standard Hungarian keyboard [ and ] are AltGr+F and AltGr+G (respectively). However, +F and +G are shortcuts for search, and +AltGr+F/+AltGr+G also brings up the search instead of indenting/outdenting.

@Tacsipacsi do you know the keyboard shortcut for indenting in Gmail in Hungarian?

The easy way to fix all of this is to unbind tab/shift+tab from VE completely as we already have CTRL+[ and CTRL+] bound (this is what GMail does).

That may be a good idea, but it doesn't magically resolve the whole issue.

When I was testing the patch, I discovered that we confuse focused node and selection anchor node in a few places, and as a result we e.g. clear the selection when you try to focus toolbar buttons (so e.g. the "bold" tool will not bold anything), or treat the surface as focused because it still has the selection, even though a toolbar button has the focus (this was mostly harmless, but it looked very confusing when two elements on the page had the focus indicator at the same time).

I tried to work that out and the latest version of https://gerrit.wikimedia.org/r/c/VisualEditor/VisualEditor/+/612422 behaves the way I would expect. But it took some significant and scary-looking changes. In particular I think we might need to check that IME behavior doesn't go crazy.

@Tacsipacsi do you know the keyboard shortcut for indenting in Gmail in Hungarian?

+[ and +]. Looks like they didn’t care much about this issue…

@matmarex, do you think the issue captured in the video below (not being able to tab between the "Internal" and "Externl" link input fields) has the same root cause as the other issues being discussed on this ticket?

Video: https://youtu.be/PiZmBcrJouM

@ppelberg to file a new ticket because it is unrelated, based on our conversation in planning. Peter will also review current approach.

@ppelberg I think that behaviour is deliberate, you are supposed to use arrow keys to change tab. CC @Volker_E who worked on the accessibility for that component in OOUI.

@Esanders will add test instructions to task description, testing should be on changing focus within VE on mobile.

The only issue is if this works on other keyboard layouts that may not have easy access to '['.

@Esanders, do we maintain documentation for the different keyboard layouts used throughout the world or is best to consult an outside source, like http://kbd-intl.narod.ru/english/layouts, for this information ? I ask this wanting to better understand the implications of this change on non-English keyboard layouts.

On deployment
This patch should NOT be deployed until @Ryasmeen has had a chance to test it.

Change 612422 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] ve.ce.Surface: Allow tabbing out of surfaces without indentation features

https://gerrit.wikimedia.org/r/612422

Change 650989 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (c20aed327)

https://gerrit.wikimedia.org/r/650989

Change 650989 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (c20aed327)

https://gerrit.wikimedia.org/r/650989

Change 651722 had a related patch set uploaded (by Esanders; owner: Esanders):
[VisualEditor/VisualEditor@master] Re-apply "ve.ce.Surface: Allow tabbing out of surfaces without indentation features"

https://gerrit.wikimedia.org/r/651722

@Esanders, do we maintain documentation for the different keyboard layouts used throughout the world

Not that I know of.

do we maintain documentation for the different keyboard layouts used throughout the world or is best to consult an outside source

https://unicode-org.github.io/cldr-staging/charts/latest/keyboards/layouts/index.html would have been my offer...

Checked the text selection, toolbar functionalities and the context cards in some scenarios on iOS. Nothing seems broken. There was one case where the page jumped down when selecting a link, but this happened so many times before intermittently that I feel that might not even be relevant to this. Other than that, I think this is good to go.

@matmarex: I have checked the page: https://www.mediawiki.org/wiki/Talk:Reading/Web/Projects/Print_Styles, mentioned in the description to check the tab out functionality, it seems it's not working there, but this patch is not deployed to mw.org yet right?

Sorry @Ryasmeen, the patch was reverted, hence the “re-apply” patch that is still in review.

Sorry @Ryasmeen, the patch was reverted, hence the “re-apply” patch that is still in review.

@Esanders: No worries! :) Just move it back to Editing QA work board when that patch is re-applied.

I updated patch https://gerrit.wikimedia.org/r/c/VisualEditor/VisualEditor/+/651722 and it's ready for code review again.

We previously reverted it, because the patch caused clicking on text inside links on the editable surface to not place the text cursor. This was caused by a workaround for a bug elsewhere in our code, that treated those links as keyboard-focusable even though they are not. The new version removes the workaround and properly fixes that bug.

Change 664355 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/DiscussionTools@master] Disable VE indentation commands to allow tabbing out of the widget

https://gerrit.wikimedia.org/r/664355

Change 664357 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/Flow@master] Disable VE indentation commands to allow tabbing out of the widget

https://gerrit.wikimedia.org/r/664357

Change 651722 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Re-apply "ve.ce.Surface: Allow tabbing out of surfaces without indentation features"

https://gerrit.wikimedia.org/r/651722

Change 664357 merged by jenkins-bot:
[mediawiki/extensions/Flow@master] Disable VE indentation commands to allow tabbing out of the widget

https://gerrit.wikimedia.org/r/664357

Change 664355 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Disable VE indentation commands to allow tabbing out of the widget

https://gerrit.wikimedia.org/r/664355

Change 664936 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (422cbd755)

https://gerrit.wikimedia.org/r/664936

Change 664936 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (422cbd755)

https://gerrit.wikimedia.org/r/664936

matmarex moved this task from Code Review to QA on the Editing-team (Kanban Board) board.
matmarex edited projects, added Editing QA; removed Patch-For-Review.

Ready for testing again (on beta cluster).

Checked the following scenarios on iOS, nothing looks broken:

  • selecting text
  • opening the toolbar
  • using the link inspector (card)

Also checked the tab out functionality on the page specified on the task description and on other pages for the following scenarios:

  • Flow new topic or reply
  • DiscussionTools new topic or reply
  • VisualEditor dialogs, e.g. the caption field in media dialog,

The only exception was of-course the editing area inside the code editor, chemical and math formula editor where pressing tab would not leave the editing area but add spaces.