Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poedit reports text such as context or errors when focused #17166

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

CyrilleB79
Copy link
Collaborator

Link to issue number:

None

Summary of the issue:

With poedit 3.5 some panes containing static text have become focusable, but their content is not reported; these panes include: the context (used with pgettext), the type of formatting (e.g. when using %s or {}) and the warnings/errors (e.g. when the source string and the translated one do not end with the same punctuation). The consequence when navigating with tab is that the user hears "pane" instead of its content.

Description of user facing changes

In poedit, one will now have the context or errors reported when tabbing to these locations.

Description of development approach

For these panes, I have implemented _get_focusRedirect so that NVDA considers the static text inside the pane to have the focus instead of the pane itself, even if the static text isn't normally focusable.

Testing strategy:

Tested tab navigation and object reporting (NVDA+tab)

Known issues with pull request:

I am just not sure if it is problematic to redirect focus to a non-focusable object.

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@coderabbitai summary

Cc @LeonarddeR, since you have recently worked on poedit module, and in case you are still available for a review.

@LeonarddeR
Copy link
Collaborator

If this is a regression in Poedit 3.5, it might be better to file an issue against Poedit.

@CyrilleB79
Copy link
Collaborator Author

I am not totally sure how poedit was before 3.5.

But I seem to record that these elements were not focusable at all. And I don't record tabbing around and hearing "pane", "pane" in previous versions of poedit.

Do you know if having these static texts focusable was part of the accessibility requests that have been recently handled in poedit? Or hasn't it anything to do with it?

In any case, I feel that having such information focused (and thus heard) during tab navigation is a plus, at least for context and warnings/errors.

If I need to ask a change in poedit rather than handling in NVDA, I do not know precisely what to ask. And poedit's author probably would expect something quite precise so that the issue be actionable.
@LeonarddeR, your insight would be appreciated.

@LeonarddeR
Copy link
Collaborator

I don't recall this either. May be more frequent Poedit users can chime in here?

@nidza07
Copy link
Contributor

nidza07 commented Sep 13, 2024

I had these panes focusable when tabbing for a very long time, actually it was one of the main things that annoyed me about switching to Poedit 3, so this PR is definitely quite useful. If anything should be re-reported to the Poedit developer, it should be the annoying issue with plural forms which doesn't allow tabbing/shift+tabbing once you are in the translation edit box, that's my other annoying issue with it. In any case, that's off topic for this PR, I mainly wanted to give feedback that the panes were there for me even before 3.5.

@CyrilleB79
Copy link
Collaborator Author

Thanks @nidza07 for the feedback. So these panels are not a regression, at least not a recent one.

Have you tried this PR?

Re plurals, since poedit v 3.5 I can tab or shift+tab out of the translation edit field of plural forms; the issue was present until version 3.4.x. Could you double check?

The only issue which remains with plural forms is vslavik/poedit#863 that I have just opened.

@nidza07
Copy link
Contributor

nidza07 commented Sep 13, 2024

I have just tried the build of this PR with the nvda.po file. I edited a string on purpose to not match punctuation, the first one "Alpha numeric input"
If I tab past the source edit field, I hear "The translation should not end with “.”.", which is already a lot better, without this PR, I just heard "panel".

If I try with a string containing a type of formatting, %s in particular, I hear "Python format" when tabbing past the list of translations, again better than the previous behaviour of just hearing "Panel".

On the other hand, in the "userGuide.xliff" file the results are a bit different:
Taking the string as an example:
"-l LOGLEVEL |--log-level=LOGLEVEL |The lowest level of message logged (debug 10, i"
The first thing I hear after tabbing from the list is "9ab444d4-78f5-44f7-b0e5-1e34d897706f"
This seems like useless clutter that ideally shouldn't even be in the tab order, but I don't think it is significantly worse with this PR, in the past I would also have an additional item, it would just be spoken as "Panel".
The results after this item are exactly the same as with the nvda.po file, I have the source edit box, I hear the warning when tabbing if there is one as the next item.

Re: Plurals, thanks for this information. I was waiting a little on upgrading to Poedit 3.5 as I wasn't yet running a NVDA version with the latest PoEdit app module, but it is definitely fixed now which is great news.

@CyrilleB79
Copy link
Collaborator Author

I have just tried the build of this PR with the nvda.po file.

Thanks for your tests.

On the other hand, in the "userGuide.xliff" file the results are a bit different: Taking the string as an example: "-l LOGLEVEL |--log-level=LOGLEVEL |The lowest level of message logged (debug 10, i" The first thing I hear after tabbing from the list is "9ab444d4-78f5-44f7-b0e5-1e34d897706f" This seems like useless clutter that ideally shouldn't even be in the tab order, but I don't think it is significantly worse with this PR, in the past I would also have an additional item, it would just be spoken as "Panel".

This hex number you hear is probably the context information coming from the xliff file. In the case of the User Guide, the context information is of no use for translators, but in the case of the GUI (nvda.po), the context information is something like "addonStore", "braille formatting symbol", etc.), which should help the translator to discriminate two same strings in different context.
In the case of the User Guide, @michaelDCurran has had to put a random number to avoid situations where strings can be the same in English but translated differently depending on the context in the target language.

So in summary, we have just to ignore it for User Guide.

@vslavik
Copy link

vslavik commented Sep 13, 2024

Poedit 3.5 updated the wxWidgets library from 3.0 to 3.2 — that's probably the cause of the "pane" thing. There were no intentional changes in focus handling there. Replacing "pane" in Poedit with actual description should be easily doable.

@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Sep 17, 2024
@CyrilleB79
Copy link
Collaborator Author

@vslavik are these items (warning/error, context) meant to be focusable? If it's a regression, do you plant to fix it soon?

From screen reader users point of view, having these items focusable and part of the tab order during keyboard navigation allows them to learn about the displayed information (e.g. context, warning/error) while navigating with the keyboard.
On the other hand, I remember that in the past, your position was to avoid making focusable controls with no user action (e.g. static text). If your position is unchanged and make these static text controls unfocusable, in any case NVDA's specific code (poedi app module) is here to guarantee that NVDA user can though notice such information when available while navigating in the list: a beep is heard and a "*" character is added in braille content to indicate an error/warning; and the warning/error is available on demand.

More generally, is this issue discussed or tracked on poedit side (if needed)? If yes, could you provide the link?

@vslavik
Copy link

vslavik commented Sep 19, 2024

@vslavik are these items (warning/error, context) meant to be focusable?

There's an active button/hyperlink in it ("ignore"), but the line itself shouldn't be focusable and it is surprising.

On the other hand, I remember that in the past, your position was to avoid making focusable controls with no user action (e.g. static text).

That's the standard guidance: TAB navigation goes through interactive elements only. Changing that would wreak havoc on non-NVDA use of keyboard... If there's a way to make them navigable for screenreaders only, I'm happy to do it. I can also always hardcode their IDs.

More generally, is this issue discussed or tracked on poedit side (if needed)? If yes, could you provide the link?

I don't think this particular one is there. The list stuff is at vslavik/poedit#696

@seanbudd
Copy link
Member

seanbudd commented Dec 2, 2024

@CyrilleB79 - what is the status of this PR? is this blocked on something on Poedits side?

# Conflicts:
#	user_docs/en/changes.md
@CyrilleB79
Copy link
Collaborator Author

@vslavik, currently in poedit, some non-interactive controls (panes) are focusable, and part of tab navigation. This is not desirable as written in #17166 (comment).
Have you a project to fix this? And if yes, an ETA? Thanks.

@CyrilleB79
Copy link
Collaborator Author

@seanbudd, I'll finally switch this PR to ready.

@vslavik probably plans to modify poedit's interface to make these fields not focusable. But there is no clear date for this change.

Merging this PR will be a plus on current poedit version. And I do not expect the changes in this PR to have any effect if/when poedit static text fields become not focusable.

@CyrilleB79 CyrilleB79 marked this pull request as ready for review December 9, 2024 10:03
@CyrilleB79 CyrilleB79 requested a review from a team as a code owner December 9, 2024 10:03
@vslavik
Copy link

vslavik commented Dec 9, 2024

@vslavik probably plans to modify poedit's interface to make these fields not focusable. But there is no clear date for this change.

The one instance of this that I am aware of (the QA labels) is fixed by vslavik/poedit@2a36c7c (in v3.5.2)

@CyrilleB79
Copy link
Collaborator Author

@vslavik thanks for answering.

I have opened vslavik/poedit#875 to describe the issue. I thought that we had already discussed before (except in this PR), but I cannot find where anymore.

@seanbudd, converting again to draft until vslavik/poedit#875 is clarified.

@CyrilleB79 CyrilleB79 marked this pull request as draft December 9, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants