Make undefined Unicode characters show up in a nicer way in braille.

Make undefined Unicode characters show up in a nicer way in braille.

Instead of a cryptic Unicode code point, this makes characters that are not
defined by the current braille table show up as dot 8, which should be
a rarely used cell in most or all braille tables.

While it might be nice to give users more information than a single 'unknown
character' dot, this is the only feasable alternative to the hexadecimal
number (\xHHHH).

BUG=321715
Committed: https://crrev.com/9cebaadae0237f7fcf7285cbb74903732ab1c194
Cr-Commit-Position: refs/heads/master@{#296424}

[Peter Lundblad, 2014-09-22 16:33:27]

plundblad@chromium.org changed reviewers:
+ dtseng@chromium.org

[David Tseng, 2014-09-22 17:53:05]

https://codereview.chromium.org/589133002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table.js
(right):

https://codereview.chromium.org/589133002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table.js:43:
function fixFilenames(tables) {
Maybe appendCommonFile? (there's nothing wrong with the existing filename).

https://codereview.chromium.org/589133002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table.js:46:
table.fileName += (',' + cvox.BrailleTable.COMMON_DEFS_FILENAME_);
Can we just load the common table once elsewhere rather than adding this to all
table filenames?

https://codereview.chromium.org/589133002/diff/1/third_party/liblouis/README....
File third_party/liblouis/README.chromium (right):

https://codereview.chromium.org/589133002/diff/1/third_party/liblouis/README....
third_party/liblouis/README.chromium:23: * Add cvox-common.cti with common
definitions for all tables mentioned in
nit: are these sroted in some order?

https://codereview.chromium.org/589133002/diff/1/third_party/liblouis/cvox-co...
File third_party/liblouis/cvox-common.cti (right):

https://codereview.chromium.org/589133002/diff/1/third_party/liblouis/cvox-co...
third_party/liblouis/cvox-common.cti:15: undefined 8
Does this mean something in any of the tables we have (to your knowledge)?
Alternatives to this would be using the question mark (?) in the particular
table chosen.

[Peter Lundblad, 2014-09-23 12:07:21]

PTAL

I went ahead and pluralized some identifiers: fileName -> fileNames etc to
make it clear that these are actually lists everywhere.

dtseng@chromium.org writes:
> 
>
https://codereview.chromium.org/589133002/diff/1/chrome/browser/resources/chr...
> File
> chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table.js
> (right):
> 
>
https://codereview.chromium.org/589133002/diff/1/chrome/browser/resources/chr...
> chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table.js:43:
> function fixFilenames(tables) {
> Maybe appendCommonFile? (there's nothing wrong with the existing
> filename).
> 
>
https://codereview.chromium.org/589133002/diff/1/chrome/browser/resources/chr...
> chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table.js:46:
> table.fileName += (',' + cvox.BrailleTable.COMMON_DEFS_FILENAME_);
> Can we just load the common table once elsewhere rather than adding this
> to all table filenames?

If you're asking whether liblouis has the functionality to do this, AFAICT, no.
The tables list is specified with each API call and I don't think that'd be
feasable anyway because order matters between definitions.

>
https://codereview.chromium.org/589133002/diff/1/third_party/liblouis/README....
> File third_party/liblouis/README.chromium (right):
> 
>
https://codereview.chromium.org/589133002/diff/1/third_party/liblouis/README....
> third_party/liblouis/README.chromium:23: * Add cvox-common.cti with
> common definitions for all tables mentioned in
> nit: are these sroted in some order?

No particular, I thought this one and the previous one belong togetehr
since they are related.  Might move the wrapper and this stuff out of
third_party at some point FWIW.

>
https://codereview.chromium.org/589133002/diff/1/third_party/liblouis/cvox-co...
> File third_party/liblouis/cvox-common.cti (right):
> 
>
https://codereview.chromium.org/589133002/diff/1/third_party/liblouis/cvox-co...
> third_party/liblouis/cvox-common.cti:15: undefined 8
> Does this mean something in any of the tables we have (to your
> knowledge)? Alternatives to this would be using the question mark (?) in
> the particular table chosen.

In my experience dots 7 and 8 are rarely used by themselves, they are even hard
to
enter using some braille keyboards (since the keys are sometimes overloaded as
Enter and Backspace, respectively).  I haven't checked though, and am sure
there's
one or two tables where dot 8 alone is used for some purpose.  I think using
question mark, while intuitive in a way, is actually worse in general than
dot 8 because it is a common character in many languages and you won't
be able to distinguish between that and 'unknown character' in many cases.
(In addition, it'd mean more work and maintainence since we'd have to check
each table for what character question mark is and potentially update as tables
change).  Hopefully users will have a braille table selected where characters
they 'care about' are represented, minimizing this issue, but obviously
no table is complete and text can contain whatever random characters.

> https://codereview.chromium.org/589133002/

-- 

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[David Tseng, 2014-09-23 17:48:42]

lgtm

https://codereview.chromium.org/589133002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table.js
(right):

https://codereview.chromium.org/589133002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table.js:19: *  
fileNames:string
nit: would be clearer if this was an array of string. Up to you though.

https://codereview.chromium.org/589133002/diff/20001/third_party/liblouis/cvo...
File third_party/liblouis/cvox-common.cti (right):

https://codereview.chromium.org/589133002/diff/20001/third_party/liblouis/cvo...
third_party/liblouis/cvox-common.cti:14: # the code point value preced by a
backslash.
nit: preceded

[Peter Lundblad, 2014-09-24 14:45:56]

dtseng@chromium.org writes:
> lgtm
> 
> 
> 
> 
>
https://codereview.chromium.org/589133002/diff/20001/chrome/browser/resources...
> File
> chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table.js
> (right):
> 
>
https://codereview.chromium.org/589133002/diff/20001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table.js:19:
> *   fileNames:string
> nit: would be clearer if this was an array of string. Up to you though.

Leaving as is since it's the actual format used by the liblouis API.

>
https://codereview.chromium.org/589133002/diff/20001/third_party/liblouis/cvo...
> File third_party/liblouis/cvox-common.cti (right):
> 
>
https://codereview.chromium.org/589133002/diff/20001/third_party/liblouis/cvo...
> third_party/liblouis/cvox-common.cti:14: # the code point value preced
> by a backslash.
> nit: preceded

Done.


Thanks,
//Peter
> https://codereview.chromium.org/589133002/

-- 

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Peter Lundblad, 2014-09-24 14:46:24]

The CQ bit was checked by plundblad@chromium.org

[commit-bot: I haz the power, 2014-09-24 14:47:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/589133002/40001

[commit-bot: I haz the power, 2014-09-24 15:47:40]

Committed patchset #3 (id:40001) as d70e4bd5cbc63c987bd5e42d5a9d01fa643cca62

[commit-bot: I haz the power, 2014-09-24 15:48:18]

Patchset 3 (id:??) landed as
https://crrev.com/9cebaadae0237f7fcf7285cbb74903732ab1c194
Cr-Commit-Position: refs/heads/master@{#296424}