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

'noname' attachment not hidden in password-protected message #5459

Closed
sosnovsky opened this issue Oct 17, 2023 · 5 comments · Fixed by #5472
Closed

'noname' attachment not hidden in password-protected message #5459

sosnovsky opened this issue Oct 17, 2023 · 5 comments · Fixed by #5472
Assignees
@martgil
Labels
actionable bug in progress
Milestone
8.5.3: Improvemen...

Comments

@sosnovsky
Copy link
Collaborator

After merging #5445 noname attachments are not hidden in password-protected messages and 2 attachments label is visible, but it should be hidden too.

Need also to add ui test checking that noname attachment and this label is hidden in password-protected messages.

before
Screenshot 2023-10-17 at 08 20 52

after
Screenshot 2023-10-17 at 09 42 31
@sosnovsky sosnovsky added this to the 8.5.2 milestone Oct 17, 2023
@martgil
Copy link
Collaborator

martgil commented Oct 20, 2023

I'll fix this now.

(For me) Test email to use:

https://mail.google.com/mail/u/[email protected]/#search/noname/FMfcgxwDrRShslZMMFjPWpzwTJxjhJfr

@martgil
Copy link
Collaborator

martgil commented Oct 20, 2023

Hello @sosnovsky, Could we treat the 'noname' as an attachment type 'hidden'?

@sosnovsky
Copy link
Collaborator Author

You mean to hide all noname attachments? But I think we initially had an issue when some non-PGP noname attachments weren't rendered correctly, as they had another content, not related to encryption. So some noname attachments should still be rendered

@martgil
Copy link
Collaborator

martgil commented Oct 20, 2023

Yes, for all noname attachments. Ok, I get it now. So I'll just hide those noname attachments for password messages and leave them as is for PGP encrypted messages. Is it correct?

@sosnovsky
Copy link
Collaborator Author

For PGP-encrypted messages it should be hidden in some cases too, as some noname files can include PGP-signatures (which should be parsed and attachment hidden), also PGP/MIME messages sometimes include noname file with PGP/MIME version identification (they're hidden now, which is correct behavior)

So hiding behavior should depend on noname attachment content, not message type (password-protected or PGP-encrypted)

This was referenced Oct 30, 2023
Merged
Closed
sosnovsky pushed a commit that referenced this issue Nov 16, 2023
@martgil
#5459 Fix visible 'noname' email attachment file (#5472)
fc781f7 
* add conditional statement for hiding noname attachment

* wip: update test

* wip: add test for password messages

* #5460 hide number of attachments

* #5460 add live gmail test

* wip: live gmail test

* wip: fix incorrect attachment count

* pr reviews: fix incorrect number of attachments

* bug fix: hide attachment container only when .pgp_pubkey is hidden

* pr reviews: include missing test on gmail.ts

* wip: revert changes on semaphore.yml

* wip: trim blank line

* pr reviews: fix bug

* wip: live-gmail-test

* wip: revert changes on semaphore.yml

* pr reviews: fix minor issues

* wip: fix test for filename

* wip: fix ui for pgp block button
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@martgil martgil

Labels
actionable bug in progress
Projects
None yet
Milestone
8.5.3: Improvements for encrypted con...
Development

Successfully merging a pull request may close this issue.

#5459 Fix visible 'noname' email attachment file FlowCrypt/flowcrypt-browser
2 participants
@sosnovsky @martgil