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.

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

Unsendable messages have frozen spinners, on old Chrome #3730

Closed
rk-for-zulip opened this issue Dec 18, 2019 · 3 comments
Closed

Unsendable messages have frozen spinners, on old Chrome #3730

rk-for-zulip opened this issue Dec 18, 2019 · 3 comments
Labels
a-compose/send Compose box, autocomplete, camera/upload, outbox, sending a-message list

Comments

@rk-for-zulip
Copy link
Contributor

rk-for-zulip commented Dec 18, 2019

Unsendable messages appear in the message list with a frozen spinner, as shown below.

Screenshot_20191217_163124

(This is a reprise, but apparently not a duplicate, of #1546.)

@rk-for-zulip
Copy link
Contributor Author

This is fine in a modern Webview; per MDN it should only fail on Chrome <43, where the animation property isn't supported.

On the other hand, the -webkit-animation property is supported as early as Chrome 18.

rk-for-zulip added a commit to rk-for-zulip/zulip-mobile that referenced this issue Dec 20, 2019
The `animation` property doesn't appear to have been available without
this prefix until Chrome 43, and we presently try to support as far
back as Chrome 37.

Fixes zulip#3730.
@gnprice gnprice added a-compose/send Compose box, autocomplete, camera/upload, outbox, sending a-message list labels Dec 31, 2019
@gnprice
Copy link
Member

gnprice commented Dec 31, 2019

Huh, is this limited to unsendable messages? Or does it apply to all messages -- so e.g. if you're offline or have a slow connection, just enough to observe the spinner for more than a moment, you see the same effect?

I see also #3731 which is about "permanently unsendable" messages; is that the same set of messages?

@gnprice
Copy link
Member

gnprice commented Dec 31, 2019

Ah I see, but also this is only on Chrome <43. Copying what I just wrote on the PR #3735 (maybe I should have made the detailed comment here in the first place; oh well):


Hmm, this is quite a fair bit of duplication: 23 lines across 8 places in the file. [Which is a perfectly reasonable way of making the fix; it's just the nature of the fix.]

Here's what we say (in js.js) about old Chrome versions:

 * * For Android, core functionality needs to work on Chrome 44.
 *   Graceful degradation is acceptable below Chrome 58. [...]
 *
 *   * Below Chrome 44, it's possible (but rare) for a user to be on a
 *     version as old as Chrome 37, which shipped with Android 5 Lollipop.
 *     We sometimes fix issues affecting those versions, only when trivial.

I think the complexity of this fix falls well above the "trivial" threshold, in terms of its maintenance cost for ongoing changes to our CSS styles.

Happily the issue is also fairly minor: it doesn't affect the actual functionality, and most users will rarely to never see it at all. So I think it's the kind of "graceful degradation" I'd be OK with at any Chrome version below 58, when the fix isn't trivial.

Thanks for looking into this! I think based on the information collected, the best path is to close the PR and issue (as WONTFIX), and turn attention to other bugs and features.

@gnprice gnprice closed this as completed Dec 31, 2019
@gnprice gnprice changed the title Unsendable messages have frozen spinners Unsendable messages have frozen spinners, on old Chrome Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-compose/send Compose box, autocomplete, camera/upload, outbox, sending a-message list
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants