Repository navigation
Kindle 1.19 support #6638
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
Merged
Merged
Kindle 1.19 support #6638
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
110281d
Fix failure to detect focus change when opening a book from the Kindl…
jcsteh 6498b80
Allow TextInfos to override getFormatFieldSpeech.
jcsteh 62f42c6
CursorManager: Refactor selection code to work for TextInfos that are…
jcsteh 8ebf365
RootProxyTextInfo: Proxy pointAtStart to the inner TextInfo. This is …
jcsteh fc4b7b0
IA2TextTextInfo: When fetching IA2 text attributes, don't go back one…
jcsteh a3a0f4e
Compound TextInfos: Don't set the name attribute on control fields fo…
jcsteh 6b67973
MozillaCompoundTextInfo: Support instantiation from NVDAObject.
jcsteh f853d44
MozillaCompoundTextInfo: Support retrieving selection even when the i…
jcsteh e5bf6bc
Various fixes in CompoundTextInfo and MozillaCompoundTextInfo:
jcsteh 472e20c
Support for new Kindle functionality introduced in Kindle 1.19, as we…
jcsteh 364b566
Document Kindle support in User Guide.
jcsteh ed51c55
User Guide clarification.
jcsteh 5f4c4bf
Kindle: In the Elements List, don't provide options for Headings and …
jcsteh 9cfc9b9
Fix failure when uninstalling add-ons that have an installTasks modul…
jcsteh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Compound TextInfos: Don't set the name attribute on control fields fo…
…r links, as this cause double speaking for quick nav. Use the content attribute on control fields for graphics, as this works well for both speech and braille.
- Loading branch information
commit a3a0f4ea4ef3dbd6bca51e43a6d5c7cbe19e0444
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jcsteh I'm trying to debug an issue where "space" in focus mode after an image in a content editable. The speech ends up as "graphic blah space" in both firefox and chrome. A space character in the
textWithFieldswhile ingetTextInfoSpeechis converted to the word "space" which originates here.I don't understand why the space is included, is there anything you can remember about this? Even test cases or something this supports that might help me avoiding a regression while fixing this?
The word "space" comes from spelling the character called from:
nvda/source/speech/__init__.py
Line 1280 in 87814b1
I'm considering whether it makes sense in this block (in
speech/__init__.py) to yield the spelling sequence afteryield speechSequence. The conditions above this are really specific to moving by one character, so if there is content in theinitialFieldsthen we don't need the space?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I recall correctly, the reason for the space is this:
For contentEditable, graphics, etc. essentially act like characters. That is, they occupy one stop with right/left arrow. However, they have no text, so we need some character as a placeholder. A space is generally what we use in this case. You can also see this in virtual buffers. For example:
data:text/html,before<hr>afterIf you repeatedly move through that with the right arrow key, you'll eventually hear "separator space". That's not ideal - it should probably just say "separator" - but we need some character in the stream.
One way to fix this that I vaguely recall considering is to use some other character (e.g. U+FFFC or U+FFFD) and explicitly "silence" that in speech and braille.
If I understand correctly, the problem with this is that it would prevent reading of spaces, say, at the start of links. For example:
data:text/html,before<pre><a href="https://nvaccess.org/"> test</a>Right arrow to the link. You should hear "link space"; the space is real and the user should know about it. If you make this change, you might only hear "link".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a more realistic example of where we don't want to lose the space:
data:text/html,<pre>This is my<ins> lovely</ins> dog.</pre>When you right arrow to the insertion, you should hear "insertion space". It's really important in this case that the user knows the space is part of the insertion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Thanks for the example!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out either removing this space or changing it to something else (eg to "testing") does not seem to have any impact on those examples, but does fix the bug I'm seeing with:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This image has the alt text "an alt", apologies that this is more confusing than I intended.
Steps to repro:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested removing the space, which in particular causes issues for routing in braille. I also tested using the object replacement character (0xFFFC), which then gets displayed in braille. Although we use the object replacement character in a few places in NVDA, generally I think it's a mistake when we know what it is acting as a stand in for. Instead it would be preferable to use type like TextInfos.ControlField to represent it and handle it's replacement at the presentation layer (braille or speech). I'm actually not sure why ControlFields don't already handle this. Also, object replacement characters could come from the content itself. Using them as a stand in can cause:
I think I'm going to have to leave this behavior as it is. I'll create a new issue, and add some comments to the code.
Ideally we would replace this space with an NVDA internal object replacement representation object, or customize ControlField to handle this. everywhere that we process field commands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See issue ""space" announced after image" #11291