-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Render each mime-part into an individual iframe #9787
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
base: master
Are you sure you want to change the base?
Conversation
4fd3901 to
2a397fe
Compare
2a397fe to
44b0165
Compare
937cb49 to
50fb5bb
Compare
46353ff to
e2038f0
Compare
program/js/app.js
Outdated
| fn = function (kr) { | ||
| ref.mailvelope_keyring = kr; | ||
| ref.mailvelope_init(action, kr); | ||
| this.afterAllContentIframesLoaded(() => { |
There was a problem hiding this comment.
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.
program/js/app.js
Outdated
| // Hide "Loading data" message. | ||
| $(iframe).siblings('.loading').hide(); | ||
| // Show remote objects notice | ||
| if (iframe.contentDocument.body.dataset.extlinks === 'true') { |
There was a problem hiding this comment.
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.
|
I gave it a try, here are some more issues:
|
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.
|
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.
Update: Scratch that, I get it now. |
Note: This breaks tests that download files
e2038f0 to
2803464
Compare
|
FTR: This won't receive any updates in the near future due to a focus shift and resource exhaustion. |
Implementing #9465
A new clean restart over the previous pull request.
TODO: