Skip to content

Conversation

@pabzm
Copy link
Member

@pabzm pabzm commented Feb 20, 2025

Implementing #9465

A new clean restart over the previous pull request.

TODO:

  • reading emails using mailvelope
  • check/change larry skin
  • Don't "wash" HTML content twice
  • Check reading emails with enigma
  • Fix hide_blockquote plugin
  • Find out why the "loading" message gets hidden so early in chromium-based browsers.

@pabzm pabzm requested a review from alecpl February 20, 2025 10:22
@pabzm pabzm self-assigned this Feb 20, 2025
@pabzm pabzm force-pushed the message-render-iframe branch 4 times, most recently from 4fd3901 to 2a397fe Compare February 21, 2025 10:47
@pabzm pabzm force-pushed the message-render-iframe branch from 2a397fe to 44b0165 Compare March 5, 2025 13:40
@pabzm pabzm changed the title Message render iframe Render each mime-part into an individual iframe Mar 5, 2025
@pabzm pabzm force-pushed the message-render-iframe branch 4 times, most recently from 937cb49 to 50fb5bb Compare March 6, 2025 10:49
@pabzm pabzm marked this pull request as draft March 7, 2025 12:28
@pabzm pabzm force-pushed the message-render-iframe branch 2 times, most recently from 46353ff to e2038f0 Compare March 21, 2025 14:45
@pabzm pabzm marked this pull request as ready for review March 21, 2025 15:20
fn = function (kr) {
ref.mailvelope_keyring = kr;
ref.mailvelope_init(action, kr);
this.afterAllContentIframesLoaded(() => {
Copy link
Member

Choose a reason for hiding this comment

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

it should be ref not this.

// Hide "Loading data" message.
$(iframe).siblings('.loading').hide();
// Show remote objects notice
if (iframe.contentDocument.body.dataset.extlinks === 'true') {
Copy link
Member

Choose a reason for hiding this comment

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

I'mk getting "TypeError: iframe.contentDocument is null" when opening any message.

@alecpl
Copy link
Member

alecpl commented Mar 23, 2025

I gave it a try, here are some more issues:

  • I see that we do not load our styles.css into the iframes. I.e. drark mode is broken, the plain text messages font is not monospace, signatures aren't greyed, blockquotes unstyled, etc.
  • messages charset is broken (for messages in charset other than utf-8).
  • the loading element disappears in onload (?), I think it's too late. I.e. sometimes I see both this element and iframe content being loaded. It does not look good. Maybe solution is to style the iframes in an "overlay" style loader, where that element is on top of the loaded iframe.
  • performance: the simplest case - opening a plain text message. I get two imap connections both of them do SELECT INBOX, UID FETCH 33469 (UID RFC822.SIZE FLAGS..., UID FETCH 33469 (BODY.PEEK[1]). Even more redundancy when loading a html message with images.
  • for performance reasons the format-change action (switching between HTML and plain text) should probably be changed to not reload the whole preview frame, but only the #messagebody element. Not a big deal for now.
  • when Enigma plugin can't decrypt a message the "loading data" message never disappears. I also think this plugin will need some other non-trivial updates.
  • small margin issue at the bottom of a preview frame, when you have a message with attached image and "Display attached images below the message" enabled.

pabzm added 10 commits April 24, 2025 15:12
This includes a new "message loading" notice without meta refresh (which
requires unsafe-inline in a CSP, which we want to avoid)
We need the information in the browser, because the
remote-objects-message is now rendered independently from the message
contents, and we need it for each message part.
The calling code replaced the $rcmail->output on the fly, which makes is
hardly testable.
Also that class was used only in the class `rcmail_action_mail_get`, and
it's a pretty thin layer on top of `rcmail_output_html`, which is not
necessary.
@pabzm
Copy link
Member Author

pabzm commented Apr 30, 2025

Thank you for the review!

The missing styling was an artefact of a rebase after the breaking changes. I fixed that now, but during that I found that displaying text attachments in the new window had additional bugs and differences to the current state of the "master" branch.

While fixing those I stumbled upon something unexpected: text/plain parts are rendered differently from the preview vs. attachment view (new window, _action=get). The former wraps it in a div.pre with styling, the latter wraps it in a pre and applies no styling.

@alecpl Can you explain why that is and/or if that needs to be kept? It would be easier to render both cases the same.

Update: Scratch that, I get it now.

@pabzm pabzm force-pushed the message-render-iframe branch from e2038f0 to 2803464 Compare July 8, 2025 06:05
@pabzm
Copy link
Member Author

pabzm commented Jul 31, 2025

FTR: This won't receive any updates in the near future due to a focus shift and resource exhaustion.

@pabzm pabzm marked this pull request as draft July 31, 2025 07:33
Sign up for free to 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.

3 participants