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

Disable pass1only for liblouis braille output #7301

Closed
leonardder opened this Issue Jun 19, 2017 · 22 comments

Comments

Projects
None yet
9 participants
@leonardder
Collaborator

leonardder commented Jun 19, 2017

Note: This issue needs an external fix at the moment, liblouis/liblouis#133 . I opened this issue to document cases why using Liblouis multipass is desired.

Steps to reproduce:

  1. Set braille output table to Dutch (Netherlands)
  2. Open notepad, and type: "1b3d5"

Expected behavior:

Output is: ⠼⠁⠠⠃⠼⠉⠠⠙⠼⠑

Actual behavior:

Output is: ⠼⠁⠃⠼⠉⠙⠼⠑
It is not clear how to read this. It is not possible to see where the numbers end and the letters start. In this example, you may be able to guess that there is a b and a d, not a 2 and a 4. However, In strings like 12cde67hij, you certainly won't be able to see where the letters start.

Technical background

NVDA sends text to the liblouis braille translator using the pass1Only mode flag. From liblouis/liblouis#133:

trace_translate , the general translation function in lou_translateString.c calls translatePass for pass2, 3 and 4. translatePass does currently not update the output positions.

Specifically, this leads to the problem that cursor routing is said to break in NVDA as the braille character to output position mapping is broken. I wasn't able to reproduce this using the example above, though.

CC @josephsl @dkager

@leonardder

This comment has been minimized.

Collaborator

leonardder commented Jun 19, 2017

Relevant information from #2439 (comment)

(Side question: why does NVDA disable the multi-pass facilities when forward translating?)

Because multi-pass breaks the input/output position mapping. I have had grand ideas of completely rewriting that position mapping code for a while, but I've never gotten to it. Arend Arends contributed a patch for this, but it hasn't been integrated yet and I believe there are some outstanding concerns

@feerrenrut

This comment has been minimized.

Contributor

feerrenrut commented Jun 19, 2017

If I understand correctly, this is blocked by liblouis/liblouis#133 however once that issue is addressed this would be a fairly high priority fix.

@leonardder

This comment has been minimized.

Collaborator

leonardder commented Jun 19, 2017

@feerrenrut: This is true. However, I recall @jcsteh saying that it is desired to have a fair amount of unit tests, especially for edge cases. As for literary braille tables, I'm only know the Dutch table pretty well, so contributions (str's) from others are more than welcome.

@jcsteh

This comment has been minimized.

Contributor

jcsteh commented Jun 19, 2017

@leonardder

This comment has been minimized.

Collaborator

leonardder commented Jun 19, 2017

When you say your example doesn't seem to break, did you test mapping in both directions; i.e. that routing routes to the correct case and also that moving the cursor with the keyboard moves the braille cursor to the right place?

Yes. For characters spanning multiple cells (e.g. numbers), cursor automatically ends up at the right most braille cell.

@dkager

This comment has been minimized.

Collaborator

dkager commented Jun 19, 2017

The Dutch table is quite well tested, but only for forward translation. We should add tests for input/output mapping. I think the lou_checkyaml tool allows for this already.

@leonardder

This comment has been minimized.

Collaborator

leonardder commented Jul 7, 2017

I tried to reproduce my str in JAWS, and it seems like they do use multi pass for liblouis.

@leonardder

This comment has been minimized.

Collaborator

leonardder commented Oct 25, 2017

@BueVest suggested to make this a GUI configurable option. Although I disagree with the idea to make this GUI configurable, I belief it should be considered to make this a hidden option. This will allow us and liblouis folks to test possible brokenness in liblouis using NVDA.

Thoughts @michaelDCurran, @dkager, @jcsteh, @josephsl and others?

@josephsl

This comment has been minimized.

Collaborator

josephsl commented Oct 25, 2017

@leonardder

This comment has been minimized.

Collaborator

leonardder commented Oct 25, 2017

@josephsl commented on 25 Oct 2017, 20:26 CEST:

let this be a volatile key (reset when NVDA restarts). Thanks.

Tend to disagree with this. Though an option in the gui is too much, this won't be a proper solution for people like @BueVest.

@BueVest

This comment has been minimized.

BueVest commented Oct 25, 2017

@zstanecic

This comment has been minimized.

Contributor

zstanecic commented Oct 25, 2017

@josephsl

This comment has been minimized.

Collaborator

josephsl commented Oct 25, 2017

@zstanecic

This comment has been minimized.

Contributor

zstanecic commented Oct 25, 2017

@leonardder

This comment has been minimized.

Collaborator

leonardder commented Oct 26, 2017

@josephsl commented on 25 okt. 2017 23:01 CEST:

Hi, or we should have a snapshot series with this option added for testing purposes.

Not intending to be headstrong, but I also tend to disagree with this one:

  1. There are currently people suffering from our use of pass1only, see #7526. Having a snapshot series requires maintenance for testers and try build creators (i.e. they can't be updated automatically).
  2. The issues with multi pass are probably quite hard to track down. It requires intensive testing from braille users, especially those using contracted braille. A hidden config option in official builds increases the likelihood that people will test this. I, for example, will from now on use multi pass by default. Pretty sure @BueVest and @zstanecic will join me in this. This is in the interest of both NVDA and Liblouis.

Side note: JAWS also uses multi pass in production.

@dkager

This comment has been minimized.

Collaborator

dkager commented Oct 26, 2017

I'm not even sure why pass1Only is a flag. What point is there in half a translation, other than working around bugs in the translator? No table I have seen so far properly "falls back" to not using multi-pass if it is disabled. So ideally, whatever is broken in liblouis should be fixed (yes, I know that's an unpopular task) and the evil flag should be removed.

@zstanecic

This comment has been minimized.

Contributor

zstanecic commented Oct 26, 2017

@BueVest

This comment has been minimized.

BueVest commented Oct 26, 2017

@leonardder

This comment has been minimized.

Collaborator

leonardder commented Oct 26, 2017

@jcsteh: Would you be able to give your view on the last few comments? I belief you were a strong advocate of using pass1only.

@michaelDCurran: Given the concerns below, I belief it might be good to merge #7702 into master before the 2017.4 release, to at least give people switching capabilities inside a release version. After that, we could considering making multipass the default for next.

Note that, even though JAWS uses this in production, we don't know whether JAWS internally accounts for multi pass failure in a different way than we do.

@michaelDCurran

This comment has been minimized.

Contributor

michaelDCurran commented Oct 26, 2017

@BueVest

This comment has been minimized.

BueVest commented Nov 2, 2017

@zstanecic

This comment has been minimized.

Contributor

zstanecic commented Nov 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment