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

Ignore "@instance..." part of username when computing status length #3392

Merged
merged 3 commits into from
Mar 13, 2023
Merged

Ignore "@instance..." part of username when computing status length #3392

merged 3 commits into from
Mar 13, 2023

Conversation

nikclayton
Copy link
Contributor

@nikclayton nikclayton commented Feb 27, 2023

In a status with a mention ("@[email protected]") only the "@foo" part should be included in the calculated status length. It wasn't, so the app was preventing people from posting statuses that should have been allowed.

Fix this.

  • Lift the length calculation code in to a separate static function (easier and faster to test)
  • Add a MentionSpan type, to reuse existing code for detecting mentions
  • Fix a bug in FakeSpannable.getSpans() (it was returning the outer type, not the wrapped inner span)
  • Add additional fast tests

The tests made sense under the components.compose.ComposeActivity package, so I also created that and moved the existing ComposeActivity tests there.

Fixes #3339

Nik Clayton added 2 commits February 26, 2023 11:16
In a status with a mention ("@foo@example.org") only the "@foo" part should
be included in the calculated status length. It wasn't, so the app was
prevening people from posting statuses that should have been allowed.

Fix this.

- Lift the length calculation code in to a separate static function (easier
  and faster to test)
- Add a `MentionSpan` type, to reuse existing code for detecting mentions
- Fix a bug in `FakeSpannable.getSpans()` (it was returning the outer type,
  not the wrapped inner span)
- Add additional fast tests

The tests made sense under the `components.compose.ComposeActivity` package,
so I also created that and moved the existing ComposeActivity tests there.

Fixes #3339
@nikclayton
Copy link
Contributor Author

nikclayton commented Feb 27, 2023

The necessity of the change to NoUnderlineURLSpan here is weird.

The symptom was that, under test, the .url property of the span was returning null, when debugging showed that it was definitely set.

I tried reproducing this behaviour in https://play.kotlinlang.org/#eyJ2ZXJzaW9uIjoiMS44LjEwIiwicGxhdGZvcm0iOiJqYXZhIiwiYXJncyI6IiIsIm5vbmVNYXJrZXJzIjp0cnVlLCJ0aGVtZSI6ImlkZWEiLCJjb2RlIjoiLyoqIFNwYW5zIHJlZmVyIHRvIHBhcnRzIG9mIHRleHQgKi9cblxuZGF0YSBjbGFzcyBTcGFuSG9sZGVyKHZhbCBzcGFuOiBBbnk/LCB2YWwgc3RhcnQ6IEludCwgdmFsIGVuZDogSW50KVxuXG5jbGFzcyBGYWtlU3Bhbm5hYmxlKHZhbCB0ZXh0OiBTdHJpbmcpIHtcbiAgICB2YWwgc3BhbnMgPSBtdXRhYmxlTGlzdE9mPFNwYW5Ib2xkZXI+KClcbiAgICBcbiAgICBmdW4gc2V0U3Bhbih3aGF0OiBBbnk/LCBzdGFydDogSW50LCBlbmQ6IEludCkgPSBzcGFucy5hZGQoU3BhbkhvbGRlcih3aGF0LCBzdGFydCwgZW5kKSlcbiAgICBcbiAgICBmdW4gPFQgOiBBbnk+IGdldFNwYW5zKHN0YXJ0OiBJbnQsIGVuZDogSW50LCB0eXBlOiBDbGFzczxUPik6IEFycmF5PFQ+IHtcbiAgICAgICAgcmV0dXJuIHNwYW5zLmZpbHRlciB7IGl0LnN0YXJ0ID49IHN0YXJ0ICYmIGl0LmVuZCA8PSBlbmQgJiYgdHlwZS5pc0luc3RhbmNlKGl0LnNwYW4pIH1cbiAgICAgICAgLm1hcCB7IGl0LnNwYW4gfVxuICAgICAgICAudG9UeXBlZEFycmF5KCkgYXMgQXJyYXk8VD5cbiAgICB9XG59XG5cblxub3BlbiBjbGFzcyBVcmxTcGFuKHByaXZhdGUgdmFsIHVybDogU3RyaW5nKSB7XG4gICAgZnVuIGdldFVSTCgpID0gdXJsXG59XG5cbm9wZW4gY2xhc3MgTWVudGlvblNwYW4odXJsOiBTdHJpbmcpIDogVXJsU3Bhbih1cmwpXG5cbmZ1biBtYWluKCkge1xuXHR2YWwgdCA9IEZha2VTcGFubmFibGUoXCJmaXJzdC1saW5rIHwgc2Vjb25kLWxpbmtcIilcbiAgICB0LnNldFNwYW4oVXJsU3BhbihcImh0dHA6Ly93d3cuZXhhbXBsZS5vcmdcIiksIDAsIDEwKVxuICAgIHQuc2V0U3BhbihVcmxTcGFuKFwiaHR0cDovL3d3dy5leGFtcGxlLmNvbVwiKSwgMTQsIDI0KVxuICAgIFxuICAgIC8vIExhdGVyIC4uLlxuICAgIFxuICAgIC8vIFByaW50IHRoZSBsaW5rcyBpbiB0aGUgdGV4dFxuICAgIHQuZ2V0U3BhbnMoMCwgdC50ZXh0Lmxlbmd0aCwgVXJsU3Bhbjo6Y2xhc3MuamF2YSkubWFwIHtcbiAgICAgICAgcHJpbnRsbihcIlVSTDogJHtpdC5nZXRVUkwoKX1cIilcbiAgICB9XG4gICAgXG4gICAgdmFsIGMgPSB0LmdldFNwYW5zKDAsIHQudGV4dC5sZW5ndGgsIFVybFNwYW46OmNsYXNzLmphdmEpLmZvbGQoMCkgeyBhY2MsIHNwYW4gLT4gc3Bhbi5nZXRVUkwoKS5sZW5ndGggfVxuICAgIHByaW50bG4oXCJVUkxzIGxlbmd0aDogJGNcIilcbn0ifQ== and couldn't.

The only difference I could think of is that URLSpan is implemented in Java, and maybe some critical type information is being lost in the toTypedArray() call in FakeSpannable.getSpans(), resulting in the null?

The problem also occurred running the tests under Robolectric.

The problem doesn't occur when the code's running in the app, only under test.

@connyduck connyduck merged commit 6dfdaec into tuskyapp:develop Mar 13, 2023
@nikclayton nikclayton deleted the 3339-character-count branch March 13, 2023 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Domain in mentions shouldn't be included in character limit
2 participants