Implement attachment-based forwarding.
Some notes:
- loadAsDraft was removed in favor of new enum loadType in QML, and callback based generic programming in C++
Details
Details
- Reviewers
cmollekopf - Maniphest Tasks
- T7024: Forward email
- Commits
- R162:d3228a289999: T7024 fix: implement attachment-based forwarding
Diff Detail
Diff Detail
- Repository
- R162 Kube
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
Great work!
The icons should be added via the copybreeze script, and some minor nitpicks inline.
| framework/src/domain/composercontroller.h | ||
|---|---|---|
| 50 | Instead of the extra class, perhaps the enum could just become part of the ComposerController? | |
| framework/src/domain/mime/mailtemplates.cpp | ||
| 879 | I think the filename can be removed entirely. Also, roundcube seems to not set a filename so let's just follow that. | |
| 887 | Those can be removed I think (or otherwise they should be qDebug) | |
| views/composer/qml/View.qml | ||
| 68 | That comment is no longer necessary since the code is much clearer now. | |
Comment Actions
- Do not set attachment name to the forwarded mail
- Move ComposerLoadType inside the ComposerController class
- Remove useless debug logs
Comment Actions
To import the icons you have to modify and run the copyBreeze script in the icons subdirectory.
| framework/src/frameworkplugin.cpp | ||
|---|---|---|
| 123 ↗ | (On Diff #27601) | Just use the enum via ComposerController instead, no need for the extra type. |
Comment Actions
Tried the patch, here's some more feedback:
- The modification of the copybreeze script is still missing.
- Not your fault, but please exclude all the mimetype icons. They are unrelated, so let's do that separately.
- I revised my opinion on the filename, please set it to subject + ".eml" ending.
- In my tests the attachment doesn't actually render (I have not figured out yet why). Probably try just doing the same like roundcube by looking at the MIME message source.
- Eventually we'll want a test in framework/src/domain/mime/tests/mailtemplatetest.cpp for the forward part as well.
Comment Actions
- Add the modifications to copyBreeze.sh, duh
- Remove mimetype icons
- Set forwarded message name to original Subject + ".eml"
- Set forwarded message attachment to "inline" for client automatic display
- Add unit test
Comment Actions
Great work!
The only remaining thing is the removal of the ComposerLoadType as noted in my inline comment (just access the enum via ComposerController).
Comment Actions
Completely remove the "ComposerLoadType" and use "ComposerController" directly for composer switching mode enum
Comment Actions
Forwarded mail still isn't displayed in clients (some clients (Evolution) even believe it's an empty message)
Comment Actions
- Properly forward the message (not an encapsulated attachment)
- Reference the thread in the new message