signalapp / Signal-iOS

A private messenger for iOS.
https://signal.org
GNU Affero General Public License v3.0
10.81k stars 3.03k forks source link

Can not read received messages using VoiceOver once in a conversation window #1437

Closed mrkiko closed 7 years ago

mrkiko commented 7 years ago

Like #1210, this issue refers to an accessibility-related problem. In particular, when entering a chat window, it's no longer possible to read a received message. VoiceOver says "text_message_accessibility_label".

mrkiko commented 7 years ago

Note: I would like to help any way I can. I already installed TestFlight.

michaelkirk commented 7 years ago

@mrkiko what localization are you using?

Are you able to read outgoing messages?

Are you able to read messages in the inbox?

mrkiko commented 7 years ago

Hi Michael, Thank you for your kind reply.

I am using Italian localizzation (iOS), the Signal App is using Italian language as well.

I am not able to read outgoing or incoming message once I open a specific conversation. Inbox? I don't know what it actually is.

I'll try to explain better the problem: you may reproduce it enabling VoiceOver on your iOS device. Gestures may be a little bit difficult to grasp at the beginning in case you never used them, but feel free to contact me.

1 - I launch the signal App from springboard. 2 - I can see:

Then I can see a list of conversations I had previously with different contacts. At the end, I can see a message saying "no messages :( tap compose to send a message or invite a friend to signal". I suspect this message is spoken anyway, wether I have new messages / not new messages or not.

Now, suppose I want to send a message to a contact with a person I already had a conversation with. At this point, I select the right conversation and tap twice (VoiceOver gesture), to open it.

Now I can see:

At this point, I can hear a sequence of text_message_accessibility_label where previously there where messages. Or, in any case, I can't read incoming or outgoing message, since when I receive them, or when I send them, they go above the unalabeled "btnAttachments blue".

do you think I may test chaning iOS system language to force signal to use English or another locale instead? Tell me what I may do and I'll try. I don't have iTunes at my disposal actually, but with a Linux box I may access apps andboxes with ifuse if needed. Thank you again.

mrkiko commented 7 years ago

So: I switched my iPhone to English. Then things change, the problem of incoming and outgoing messages go away. Still: 1 - VoiceOver will read all messages as if they all where incoming ones: so I can't distinguish between messages I sent and answers from my contact. I may deduce it from the words, but the iPhone doesn't read them properly now. 2 - On the conversations list window, voiceOver will not read contact names even if they're saved in the phonebook: and are infact read correctly if localizzation is Italian.

Thank you for your help. If there are any problem, let me know.

mrkiko commented 7 years ago

BTW - it would be great if VoiceOver could read message receipts as it does in iMessage / WA? I am excplicitly mentioning other apps here just to note it's possible from an implementation point of view.

mrkiko commented 7 years ago

Sorry for the verbosity... Another note: once exiting from a conversation and returning to the conversation list window, VoiceOver seems to read even contact names properly.

MarcoZehe commented 7 years ago

There seem to be two parts to this issue:

  1. Some localizations seem to have errors. German, for example, reads fine, except for issue 2 below. Italian seems to be broken, as the original report indicates.
  2. The issue, if the messages read correctly, is that every message is read as if it came from the contact, not from oneself. So if @michaelkirk and I were contacts, on my end, the conversation would sound like this:

"Michael Kirk: Hi Marco, how's it going?" "Michael Kirk: Hi Michael, fine, thanks, how about you?"

VoiceOver does not indicate that the second message is actually sent by me. So the label for messages the current Signal user sends would need to be adjusted to deal with this. Could be either the name, or the word "Me" as the sender.

Same goes for the Inbox list, by the way, the message always sounds like it is sent from the contact, even though the last message shown might actually be the one sent by the Signal user.

There are probably more items missing that VoiceOver should read for each message, like the date received/sent/read, the different status whether a message was sent, delivered, read. I am blind myself, so cannot tell whether Signal actually has icons for these statuses. Or communicates them via color.

Hope this explains a few things further!

mrkiko commented 7 years ago

Thank you ver much to help me out clarifying things, and in general for writing.

Yeah, I can confirm this situation, even if I used the english translation to test this out, which seems to exhibit the same issues. And... a question: don't you think it's wrong when VoiceOver reads "no messages :/ tap compose..." even if there are unread messages? I had the impression Signal hides this message away with some tricks, but VoiceOver reads it anyway. Or I may be totally wrong and this is how it's supposed to work, so I should understand how it works.

Thank you all, for everything.

roostr commented 7 years ago

I'm looking into this issue today.

I have a fix for the issue where it always says the message is from the other party, and I'm looking into the localization-specific issues.

roostr commented 7 years ago

Getting rid of "no messages, tap to compose" in VO is also easy, I have a patch for that one too.

I'll do a PR later, once I review and address the other issues in this thread.

michaelkirk commented 7 years ago

Great, thank you @roostr.

If an issue appears in some localization and not others, it might be that the localization is missing from our upstream dependency: https://github.com/jessesquires/JSQMessagesViewController

roostr commented 7 years ago

Sounds good, I'll keep that in mind.

Is there a place where I can sign an authorization for my contributions to be used @michaelkirk? Couldn't find one. This is my first time contributing.

michaelkirk commented 7 years ago

You’ll find one when you open your PR.

Or you can do it before: https://whispersystems.org/cla/ https://whispersystems.org/cla/

On Nov 14, 2016, at 1:35 PM, Russ Shanahan notifications@github.com wrote:

Sounds good, I'll keep that in mind.

Is there a place where I can sign an authorization for my contributions to be used @michaelkirk https://github.com/michaelkirk? Couldn't find one. This is my first time contributing.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/WhisperSystems/Signal-iOS/issues/1437#issuecomment-260421091, or mute the thread https://github.com/notifications/unsubscribe-auth/AANP4UzDsrD2KPhd9tEKHYSDWvlBZc8Aks5q-KoLgaJpZM4KpPtA.

roostr commented 7 years ago

@michaelkirk was right, this is related to JSQMessagesViewController's localization support. There are three different categories of localization support in JSQMVC:

  1. Signal and JSQMVC both fully support a language (eg. English)
  2. Signal supports a language, JSQMVC doesn't support it (eg. Croatian)
  3. Signal supports a language, JSQMVC has a partial localization that's missing accessibility entries (eg. Italian)

Category 1 is fine. Category 2 is acceptable, because JSQMVC finds the localization file completely missing and falls back to its base localization (which is fine, it's just a formatting string). Category 3 is the one causing issues here, because JSQMVC thinks it has a localization pack, fails to find the the entry for the accessibility entry, and returns the localization key. Ideally, it would instead return the entry from the base translation.

The value we're trying to lookup is just "%@: %@", used to format "Name: Message." Falling back on the base localization is going to make a big improvement for many i18n users, as reading the messages is a big part of the Voiceover experience.

I have a workaround for JSQMVC to lookup a missing key in the base localization. I'll submit a PR and update here when it gets accepted.

This is the fix:

+ (NSString *)jsq_localizedStringForKey:(NSString *)key
{
    NSString *value = NSLocalizedStringFromTableInBundle(key, @"JSQMessages", [NSBundle jsq_messagesAssetBundle], nil);

    if ([value isEqualToString:key]) {
        // This translation is partial, and the key we're looking up is missing
        // Fall back to the Base localization
        if (![[[NSLocale preferredLanguages] objectAtIndex:0] isEqualToString:@"en"]) {
            NSString *baseTranslationPath = [[NSBundle jsq_messagesAssetBundle] pathForResource:@"Base" ofType:@"lproj"];
            NSBundle *baseTranslationBundle = [NSBundle bundleWithPath:baseTranslationPath];
            value = [baseTranslationBundle localizedStringForKey:key value:@"" table:@"JSQMessages"];
        }
    }

    return value;
}
roostr commented 7 years ago

https://github.com/jessesquires/JSQMessagesViewController/pull/1904

mrkiko commented 7 years ago

I can only say thank you to all of you helping, and to all the projecti nvolved, to which I wish my word to arrive. You're doing something great, and benefitting users that, where the'll be, they'll be able to use the Signal Private Messenger app. Thank you very much for your work and involvement; as always, feel free to contact me in any case. Enrico

roostr commented 7 years ago

Happy to help out @mrkiko.

@michaelkirk, the most significant improvement for this issue depends on an upstream merge on JSQ, and that hasn't had activity since the PR was created. I'll continue monitoring it and do what I can to help it through the process. If you have a release coming up, it still hasn't gotten merged upstream & you'd like to ship with the fix, then drop me a line and I'll take care of making a PR for you that rewires CocoaPods to point to a fixed fork.

michaelkirk commented 7 years ago

Feel free to submit a PR to our fork if it hasn't been incorporated upstream yet. https://github.com/WhisperSystems/JSQMessagesViewController

roostr commented 7 years ago

Submitted PR #8. Let me know if you'd also like a PR to update Signal-iOS's Podfile to point to it.

mrkiko commented 7 years ago

As might be expected, the new update fixes "issue #2", but not the "#1" one. In fact, Italian translation continues to exhibit the same behavior: making text messages unreadable when in a conversation. So, this was meant only to be a status update. Thank you all guys for the attention, the responsiveness and the great work. and for all.

mrkiko commented 7 years ago

Update installed, actually no changes as you may expect. PS.: I didn't test previous update for issue #2 resolution, sorry guys. Thank you again for your great work.

mrkiko commented 7 years ago

Sorry guys. I don't want to spam, still I would like to let you know I am going on testing updates. The problem isn't solved yet, but I would like to say Marry Christmas to all of you. Thank you for your great work and everything.

mrkiko commented 7 years ago

The problem is still reproducible in the current version of Signal for iOS with current iOS version. Maybe it would be useful to implement a way to tell the app to use a different language than the system current one? thank you very much again.

mrkiko commented 7 years ago

Hello guys. Can we definitely start using the modified fork of the needed code? Very kindly and Nicely, Russ Shanahan helped me and understood the problem, fixing it in the affected JSQMessageViewController component. Now some time has passed, and without this fix the App is near-unusable with VoiceOver when the phone is using Italian Language. Can we act somehow? At least by providing a settings to override system language if using a JSQMessageViewController fork should be avoided if at all possible. Thank you very much for your work guys, and sorry for me being a little bit harsh.

mrkiko commented 7 years ago

This issue manifests even with Audio Memos.

michaelkirk commented 7 years ago

A pull request for this is open here: https://github.com/WhisperSystems/Signal-iOS/pull/2344/files

michaelkirk commented 7 years ago

This should be fixed in the next release. https://github.com/WhisperSystems/Signal-iOS/pull/2344

Thanks for your patient persistence.