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.

Sign up for GitHub

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

Jump to bottom

Messages with whitespace-only topics should not be sent #3743

Closed
rk-for-zulip opened this issue Dec 30, 2019 · 4 comments · Fixed by #3748 or #3859
Closed

Messages with whitespace-only topics should not be sent #3743

rk-for-zulip opened this issue Dec 30, 2019 · 4 comments · Fixed by #3748 or #3859
Labels
a-compose/send Compose box, autocomplete, camera/upload, outbox, sending P1 high-priority

Comments

@rk-for-zulip
Copy link
Contributor

It should not be possible to attempt to send a message with a topic consisting only of whitespace.

Note: this bug interacts with #3731. If an attempt is made to post such a message, then – while such messages remain in the outbox – any future messages with empty topic will be unified with that topic, instead of being given the topic (no topic) as they ordinarily would.
@rk-for-zulip rk-for-zulip changed the title Attempts to send messages with whitespace-only topics should fail Messages with whitespace-only topics should not be sent Dec 30, 2019
@rk-for-zulip rk-for-zulip mentioned this issue Dec 30, 2019
Open
@gnprice gnprice added the a-compose/send Compose box, autocomplete, camera/upload, outbox, sending label Dec 31, 2019
hashirsarwar added a commit to hashirsarwar/zulip-mobile that referenced this issue Jan 1, 2020
@hashirsarwar
ComposeBox: Prevent message with whitespace-only topic from sending.
cd12673 
Fixes zulip#3743.
This trims the topic string whenever the message text input is focused.
Apart from preventing whitespace-only topics from sending, it will also
remove whitespace from the start and end of the topic string like Zulip
web version.
@hashirsarwar hashirsarwar mentioned this issue Jan 1, 2020
Merged
hashirsarwar added a commit to hashirsarwar/zulip-mobile that referenced this issue Jan 1, 2020
@hashirsarwar
ComposeBox: Don't send message with whitespace-only topic.
bb5b6dd 
Fixes zulip#3743.
This trims the topic string whenever the message text input is focused.
Apart from preventing whitespace-only topics from sending, it will also
remove whitespace from the start and end of the topic string like Zulip
web version.
hashirsarwar added a commit to hashirsarwar/zulip-mobile that referenced this issue Jan 1, 2020
@hashirsarwar
ComposeBox: Don't send message with whitespace-only topic.
118f955 
Fixes zulip#3743.
This trims the topic string whenever the message text input is focused.
Apart from preventing message with whitespace-only topic from sending,
it will also remove whitespace from the start and end of the topic string
like Zulip web version.
@chrisbobbe chrisbobbe mentioned this issue Jan 7, 2020
Closed
hashirsarwar added a commit to hashirsarwar/zulip-mobile that referenced this issue Jan 11, 2020
@hashirsarwar
ComposeBox: Don't send message with whitespace-only topic.
1bda95e 
Fixes zulip#3743.
This disables the floating button until the valid topic
string is inserted.
rk-for-zulip added a commit to rk-for-zulip/zulip-mobile that referenced this issue Jan 23, 2020
@rk-for-zulip
Revert "ComposeBox: Don't send message with whitespace-only topic."
3124183 
This reverts commit a2d6793.

Fixes zulip#3820 (Send button never enabled in PMs), which was a regression
induced by the reverted commit. Reopens zulip#3743.
@rk-for-zulip rk-for-zulip mentioned this issue Jan 23, 2020
Merged
@gnprice
Copy link
Member

gnprice commented Jan 23, 2020

Reopened by the revert in #3837.

@gnprice gnprice reopened this Jan 23, 2020
@gnprice
Copy link
Member

gnprice commented Jan 23, 2020

The original fix #3748 caused #3820. See #3821 (comment) for a bit more discussion on how the logic might work here.

@gnprice
Copy link
Member

gnprice commented Jan 31, 2020

Hmm. When the topic input is completely empty, we allow sending the message -- the server just turns the topic into "(no topic)". That mirrors the webapp.

Seems like when the topic input consists of whitespace, we should just treat that as an empty topic: you can send just fine, but we'll strip the whitespace, and the server will turn the topic into "(no topic)". More generally, we should strip leading and trailing whitespace in the topic.

Reassuringly, that also seems to be what the webapp does, so we'd be matching its behavior.

agrawal-d added a commit to agrawal-d/zulip-mobile that referenced this issue Feb 1, 2020
@agrawal-d
compose box: Trim leading and trailing whitespace in topic.
aba56b3 
Trim the leading and trailing whitespace from topic while
sending a message in the ComposeBox. A consequence of this is that
a whitespace-only topic gets converted to '(no topic)'.

Fixes zulip#3743.
@agrawal-d agrawal-d mentioned this issue Feb 1, 2020
Merged
agrawal-d added a commit to agrawal-d/zulip-mobile that referenced this issue Feb 1, 2020
@agrawal-d
compose box: Trim leading and trailing whitespace in topic.
dcd116f 
Trim the leading and trailing whitespace from topic on sending a
message in the ComposeBox. A consequence of this is that a
whitespace-only topic gets converted to '(no topic)'.

Fixes zulip#3743.
agrawal-d added a commit to agrawal-d/zulip-mobile that referenced this issue Feb 1, 2020
@agrawal-d
compose box: Trim leading and trailing whitespace in topic.
7651731 
Trim the leading and trailing whitespace from topic when used by
`getDestinationNarrow()`. A consequence of this is that a
whitespace-only topic gets converted to '(no topic)'.

Fixes zulip#3743.
@gnprice gnprice mentioned this issue Feb 3, 2020
Open
@gnprice
Copy link
Member

gnprice commented Feb 3, 2020

Bumping priority because this is one of the handful of most frequent errors in our Sentry logs.

rk-for-zulip pushed a commit that referenced this issue Feb 4, 2020
@agrawal-d @rk-for-zulip
compose box: Trim leading and trailing whitespace in topic.
ae709a5 
Trim the leading and trailing whitespace from topic when used by
`getDestinationNarrow()`. A consequence of this is that a
whitespace-only topic gets converted to '(no topic)'.

Fixes #3743.
Maskedman99 pushed a commit to Maskedman99/zulip-mobile that referenced this issue Feb 11, 2020
@agrawal-d @Maskedman99
compose box: Trim leading and trailing whitespace in topic.
8f330a5 
Trim the leading and trailing whitespace from topic when used by
`getDestinationNarrow()`. A consequence of this is that a
whitespace-only topic gets converted to '(no topic)'.

Fixes zulip#3743.

docs/release: Submit early for App Store review.

This saves some latency.  If we had a lot of beta releases we never
send to prod, there might be value in skipping App Store review for
those, but we don't.

UI description is a bit approximate because I don't have it in front
of me.  When I do the next release, I'll be able to see the UI again
and can fill in the exact text of things.
Maskedman99 pushed a commit to Maskedman99/zulip-mobile that referenced this issue Feb 12, 2020
@agrawal-d @Maskedman99
compose box: Trim leading and trailing whitespace in topic.
a399314 
Trim the leading and trailing whitespace from topic when used by
`getDestinationNarrow()`. A consequence of this is that a
whitespace-only topic gets converted to '(no topic)'.

Fixes zulip#3743.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
a-compose/send Compose box, autocomplete, camera/upload, outbox, sending P1 high-priority
Projects
None yet
Milestone
No milestone
2 participants
@gnprice @rk-for-zulip