Skip to content

#5308 Fix the error when sending password-protected emails that contain emojis - #5321

Merged
sosnovsky merged 25 commits into
masterfrom
issue-5308-fix-email-sending-issue-for-recipient-email-containing-emoji
Oct 3, 2023
Merged

#5308 Fix the error when sending password-protected emails that contain emojis#5321
sosnovsky merged 25 commits into
masterfrom
issue-5308-fix-email-sending-issue-for-recipient-email-containing-emoji

Conversation

@martgil

@martgilmartgil commented Jul 23, 2023

Copy link
Copy Markdown
Collaborator

This PR adds .stripEmojis() to strip out emojis of the recipient contact's name.

close#5308


Tests(delete all except exactly one):

  • Tests added or updated

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@martgil
martgil requested a review from sosnovsky as a code ownerJuly 23, 2023 09:26
@martgil
martgil marked this pull request as draft July 23, 2023 09:26
@martgilmartgil changed the title #5308 Fix the error when sending password-protected emails that contain emojis.#5308 Fix the error when sending password-protected emails that contain emojisJul 23, 2023
@martgil
martgil marked this pull request as ready for review July 29, 2023 06:53
@martgil

Copy link
Copy Markdown
CollaboratorAuthor

Manual test:

Screencast.from.2023-07-29.14-57-12.webm

@martgil

Copy link
Copy Markdown
CollaboratorAuthor

@sosnovsky ready for review. thank you.

@sosnovskysosnovsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well 👍 Just a couple small improvements needed

Comment threadextension/js/common/platform/xss.ts Outdated
Comment threadextension/chrome/elements/compose-modules/compose-input-module.ts Outdated
@martgil
martgil marked this pull request as draft July 31, 2023 10:51
Comment threadextension/js/common/platform/xss.ts Outdated
@martgil
martgil marked this pull request as ready for review August 2, 2023 07:24
@martgil
martgil requested a review from sosnovskyAugust 2, 2023 07:24
@martgil

Copy link
Copy Markdown
CollaboratorAuthor

ready for review again. thank you.

@martgil
martgil marked this pull request as draft August 2, 2023 08:10
@martgil

Copy link
Copy Markdown
CollaboratorAuthor

marking back to draft state a better solution can be discussed at #5308

@martgil
martgil marked this pull request as ready for review September 27, 2023 11:43
@martgil

Copy link
Copy Markdown
CollaboratorAuthor

@sosnovsky Ready for review. thank you!

Comment threadextension/js/common/platform/xss.ts
Comment threadextension/chrome/elements/compose-modules/compose-input-module.ts Outdated
Comment threadtest/source/tests/compose.ts
@martgil
martgil marked this pull request as draft September 28, 2023 11:50
@martgil
martgil marked this pull request as ready for review September 30, 2023 11:58
@martgil

Copy link
Copy Markdown
CollaboratorAuthor

@sosnovsky ready for review. thank you!

Comment threadextension/chrome/elements/compose-modules/compose-recipients-module.ts Outdated
Comment threadtest/source/tests/compose.ts Outdated
@martgil
martgil requested a review from sosnovskyOctober 2, 2023 09:27
Comment threadextension/chrome/elements/compose-modules/compose-recipients-module.ts Outdated
Comment threadextension/chrome/elements/compose-modules/compose-recipients-module.ts Outdated
Comment threadextension/chrome/elements/compose-modules/compose-recipients-module.ts Outdated
Comment threadextension/chrome/elements/compose-modules/compose-recipients-module.ts Outdated
Comment threadextension/chrome/elements/compose-modules/compose-recipients-module.ts Outdated
Comment threadextension/js/common/api/account-servers/external-service.ts
@martgil
martgil requested a review from sosnovskyOctober 3, 2023 04:38
Comment threadtest/source/mock/google/strategies/send-message-strategy.ts Outdated
@martgil

Copy link
Copy Markdown
CollaboratorAuthor

@sosnovsky This should be all good and ready to merge. thanks!

@martgil
martgil requested a review from sosnovskyOctober 3, 2023 07:08

@sosnovskysosnovsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, all good now 👍

@sosnovsky
sosnovsky merged commit c75fca5 into masterOct 3, 2023
@sosnovsky
sosnovsky deleted the issue-5308-fix-email-sending-issue-for-recipient-email-containing-emoji branch October 3, 2023 07:09
Sign up for freeto 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.

Unable to send password messages if the contact name retrieved from contacts.google.com includes emoji

2 participants

@martgil@sosnovsky