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

Add image support to the text editor #6935

Merged
merged 19 commits into from
Sep 2, 2022
Merged

Conversation

kesselb
Copy link
Contributor

@kesselb kesselb commented Jul 26, 2022

Fixes #2733
Fixes #2052

Changes

  • TextEditor: Rework the data flow between Composer and TextEditor component.
  • TextEditor: Add editorExecute to call ckeditor plugins externally.
  • Signature: Schema is defined as $container now. Should allow user to edit the signature in message composer.
  • Signature: Run signature command only on alias change / compose new message.
  • Backend: Add parser for DataUri.
  • Backend: Load, modify and save HTML messages via DomDocument.
  • Backend: Component to build a multipart/related message

To do

  • Testing with Mailvelope
  • Convert base64 image to attachment
  • Evaluate isHtml flag for drafts when loading a message with a different format then account default (rich vs plain)
  • Set editor mode only for new messages
  • Check DomDocument
  • Debug: mail: setting TextEditor contents to <undefined>
  • Backend: Create a multipart/related message for html, text and inline images
  • Tests (onAliasChange)
  • Align list of allowed mime types (ckeditor / html purifier)

To discuss

  • Have account A (plain text) and account B (rich text). What should happen switching from rich text to plain text?
  • Use the new HTML2Text dependency for html-to-text conversion instead of horde filter?
  • Caption/Alt for Image? A possible use case is the attachment name
  • Add a warning for images > 128kb? (big base64 encoded images make the editor laggy and slow down sending)
  • ca38fb7 add debounce for onInput event

Test cases

1. Open draft

  • New message
  • Insert image
  • Wait for draft saved
  • Close editor
  • Open drafts folder
  • Open message again

Expected result: Image is there. It's possible to add further images, delete existing images and write more text.

2. Write message

  • New message
  • Insert image
  • Send unencrypted

Expected result: Message is in the outbox. Image is there. A multipart/related email is sent. K-9, Thunderbird, Roundcube show the message properly.

To rebase the branch with main

  • Run interactive rebase
  • Drop commit "Add ckeditor upload and image as dependency"
  • npm install @ckeditor/ckeditor5-image @ckeditor/ckeditor5-upload @ckeditor/ckeditor5-vue2@^3.0.1
  • git add package.json package-lock.json
  • git commit --signoff -m "Add ckeditor upload and image as dependency"

@miaulalala
Copy link
Contributor

miaulalala commented Jul 26, 2022

oof

nice title though

@kesselb kesselb force-pushed the enh/2733/html-signatures branch 5 times, most recently from 6d6f76e to f6d9ad1 Compare August 16, 2022 14:32
@kesselb kesselb force-pushed the enh/2733/html-signatures branch 5 times, most recently from ddd042c to 3c542a1 Compare August 24, 2022 21:10
@kesselb kesselb marked this pull request as ready for review August 24, 2022 21:19
@kesselb kesselb self-assigned this Aug 24, 2022
@ChristophWurst
Copy link
Member

Nitpick: the image tool is very hidden
Bildschirmfoto vom 2022-08-26 16-12-40

@ChristophWurst
Copy link
Member

  • BUG weird behavior with image in signature but plain editing as default
  1. Set the account to plain text emails
  2. Add an image to your signature
  3. Compose a message
  4. Switch to rich text

Expected: the image appears
Actual: the image does not show

@ChristophWurst
Copy link
Member

✔️ Successfully tested embedding an image into a new HTML message
✔️ Successfully tested embedding an image into my signature and sending an HTML message

@ChristophWurst
Copy link
Member

Have account A (plain text) and account B (rich text). What should happen switching from rich text to plain text?

We answered that, right? If there is content, the editing mode won't switch with the selection of another account. If the body is empty we switch modes according to account defaults.

@ChristophWurst
Copy link
Member

Add a warning for images > 128kb? (big base64 encoded images make the editor laggy and slow down sending)

Definitely. Does CKEditor have an option for this or allow us to hook into the selection?

@ChristophWurst ChristophWurst changed the title 🖼️ Add image support to the text editor Aug 30, 2022
@ChristophWurst
Copy link
Member

Adjusted the title so it can be found in the browser history

@ChristophWurst
Copy link
Member

Add a warning for images > 128kb? (big base64 encoded images make the editor laggy and slow down sending)

Let's do this as a follow-up so the basic image support can be part of this week's RC 👍

src/components/Composer.vue Outdated Show resolved Hide resolved
src/components/TextEditor.vue Outdated Show resolved Hide resolved
isLimit prevent editing the current signature element.

The CKEditor documentation says that a limit element is something like an image caption and adds boundaries to some
internal actions (e.g a selection does not end outside or delete does not delete the area).

Signature is a collection of related blocks. Make our signature schema behave like a $container is eventually the better choice here.

Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: Daniel Kesselberg <[email protected]>
1) Adjust editor mode on alias change

Changing an alias is simple when current and new alias use the same editor mode.
Update alias object and trigger the signature plugin.

When the current alias uses text but the new alias richtext we automatically change the editor mode to html.
TextEditor uses :key="editorMode" to assign the instance to an editorMode. On :key change the old instance is destroyed and a new instance created.
This emit another onEditorReady event and will trigger the signature plugin. To replace an existing signature changeSignature is true and set TRIGGER_CHANGE_ALIAS for the signature command.

2) Add warning on disable formatting

When you compose a richtext message and disable formatting a warning is shown that formatting is lost.
Enable formatting ActionCheckbox is now replaced by buttons with icons.

Signed-off-by: Daniel Kesselberg <[email protected]>
Otherwise it's not possible to open a draft with a webp image

Signed-off-by: Daniel Kesselberg <[email protected]>
DOMPurify.sanitize may re-order the attributes.

For example: <img src="data:image/png;base64,..." alt="Test" /> to <img alt="Test" src="data:image/png;base64,...">>

CKEditor always adds the alt attribute after src attribute. Internal (CKEditor) and external (TextEditor) are different and every keystroke will emit a second event.

Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: Daniel Kesselberg <[email protected]>
DOMPurify.sanitize may take a while.
Save some sanitize executions by moving the sanitization to an earlier stage.

Signed-off-by: Daniel Kesselberg <[email protected]>
@kesselb
Copy link
Contributor Author

kesselb commented Sep 2, 2022

ca38fb7 add debounce for onInput event

Resolved via ea90bc1

@ChristophWurst ChristophWurst added this to the v2.0.0 milestone Sep 2, 2022
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Tested & works

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to load images in signatures Allow image upload in HTML editor
3 participants