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

TF-2602 Fix charset is not set when I attach a text/plain attachment #2634

Merged
merged 3 commits into from
Sep 17, 2024
Merged

TF-2602 Fix charset is not set when I attach a text/plain attachment #2634

merged 3 commits into from
Sep 17, 2024

Conversation

dab246
Copy link
Member

@dab246 dab246 commented Feb 23, 2024 •
edited
Loading

Issue

#2602

Solution

Use flutter_charset_detector to detect charset of the text file.

Demo

Screen.Recording.2024-03-19.at.08.47.31.online-video-cutter.com.mp4

Log

Screenshot 2024-02-26 at 13 16 24

Related

We can review this PR to help it quickly merge

@github-actions GitHub Actions
Copy link

This PR has been deployed to https://linagora.github.io/tmail-flutter/2634.

@dab246 dab246 requested a review from chibenwa February 26, 2024 06:47
@dab246
Copy link
Member Author

dab246 commented Feb 29, 2024 •
edited
Loading

Hi BE team cc @chibenwa @quantranhong1999 , I found a problem:

Desc:

  1. I have set charset for attachment files with mimeType=text/plain when sending emails and it was successful.
  2. But when opening the received email, in the attachment the charset attribute is set by the jmap server to default us-ascii.
  • Payload Email/set:
307682918-0070fe97-43c7-4e0a-b86d-ac8c0309a618
  • Response Email/get

Screenshot 2024-02-29 at 14 12 07

Question:

Is it wrong if I pass charset like that?. Does the server accept attachment charset changes?

@dab246 dab246 mentioned this pull request Mar 4, 2024
Closed
@chibenwa
Copy link
Member

chibenwa commented Mar 4, 2024

The contentype shall be specified when uploading the blob:

Request method:	POST
Request URI:	http://localhost:34833/upload/29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6
Proxy:			<none>
Request params:	<none>
Query params:	<none>
Form params:	<none>
Path params:	<none>
Headers:		accept=application/json; jmapVersion=rfc-8621
				Content-Type=text/plain; charset=US_ASCII
Cookies:		<none>
Multiparts:		<none>
Body:
[49, 50, 51, 52, 53, 54, 55, 56, 57, 13, 10...


Likely the Email/set charset should be able to override the one of the upload though...

@chibenwa
Copy link
Member

chibenwa commented Mar 4, 2024

apache/james-project#2081

@dab246 dab246 force-pushed the maintain-v0.11.3/tf-2602-charset-not-set-when-attach-text-plain-attachmnet branch from a5d0f76 to ce1d975 Compare March 4, 2024 09:11
@dab246 dab246 force-pushed the maintain-v0.11.3/tf-2602-charset-not-set-when-attach-text-plain-attachmnet branch from ce1d975 to 28d7417 Compare March 19, 2024 01:50
@dab246
Copy link
Member Author

dab246 commented Mar 19, 2024

  • Tested again and it is working well on preprod server after BE release.
  • We should review and merge it @hoangdat
Screen.Recording.2024-03-19.at.08.47.31.online-video-cutter.com.mp4

@hoangdat hoangdat changed the base branch from v0.11.3-patch4-dev to v0.11.3-patch4-dev-features March 19, 2024 02:20
@dab246 dab246 force-pushed the maintain-v0.11.3/tf-2602-charset-not-set-when-attach-text-plain-attachmnet branch from 28d7417 to ece7e4a Compare April 9, 2024 04:09
@hoangdat hoangdat changed the base branch from v0.11.3-patch4-dev-features to v0.11.3-patch4-dev May 31, 2024 09:36
@tddang-linagora
Copy link
Contributor

@dab246 Should this PR be rebased to refactor or we can review it at current state?

@dab246 dab246 changed the base branch from v0.11.3-patch4-dev to refactor September 4, 2024 05:39
@dab246 dab246 force-pushed the maintain-v0.11.3/tf-2602-charset-not-set-when-attach-text-plain-attachmnet branch from ece7e4a to cb089f5 Compare September 4, 2024 05:39
@dab246
Copy link
Member Author

dab246 commented Sep 4, 2024

@dab246 Should this PR be rebased to refactor or we can review it at current state?

Rebase on refactor. Please review again.

@dab246 dab246 force-pushed the maintain-v0.11.3/tf-2602-charset-not-set-when-attach-text-plain-attachmnet branch from cb089f5 to d589f39 Compare September 4, 2024 07:44
tddang-linagora
tddang-linagora approved these changes Sep 5, 2024
View reviewed changes
core/lib/utils/file_utils.dart Outdated Show resolved Hide resolved
@hoangdat
Copy link
Member

hoangdat commented Sep 17, 2024 •
edited by dab246
Loading

  • @dab246 please rebase and verify Memory leak for this PR. Please check it in both mobile and web.

@dab246 dab246 force-pushed the maintain-v0.11.3/tf-2602-charset-not-set-when-attach-text-plain-attachmnet branch from d589f39 to c825855 Compare September 17, 2024 08:24
@dab246
Copy link
Member Author

dab246 commented Sep 17, 2024 •
edited
Loading

  • @dab246 please rebase and verify Memory leak for this PR. Please check it in both mobile and web.

Memory leak

No leak

  • Web:
Screen.Recording.2024-09-17.at.15.35.09.mov
  • Mobile:
Screen.Recording.2024-09-17.at.15.22.04.mov

@dab246 dab246 requested a review from hoangdat September 17, 2024 08:27
@hoangdat hoangdat merged commit 2739a28 into refactor Sep 17, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tddang-linagora tddang-linagora tddang-linagora approved these changes

@nqhhdev nqhhdev Awaiting requested review from nqhhdev

@hieutbui hieutbui Awaiting requested review from hieutbui

@sherlockvn sherlockvn Awaiting requested review from sherlockvn

@chibenwa chibenwa Awaiting requested review from chibenwa

@hoangdat hoangdat Awaiting requested review from hoangdat

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dab246 @chibenwa @tddang-linagora @hoangdat