gauteh
commented
1 year ago Thank you. The idea with the iframe was that it was easier to separate the content from the other content, so that it would be possible to disable javascript etc and the content of one email would not be able to access the other content.
In my other failed attempt at redoing astroid in typescript / javascript / rust (whatever runs in the browser) I had to give that up, and rather sanitize the email. Since this is such a common problem I think there are good packages for doing that, but maybe not in C++.. ?
https://github.com/gauteh/ragnarok/blob/master/astroid/src/components/thread-view/PartView.tsx#L50
( I see that I somehow managed to use iframe's here after all -- it is obviously not possible to disable javascript in that case )
rothn
Consider this a WIP / broken change. It's enough to fix issues relating to the removal of frame flattening in #720 for me that does not detriment the ability for Astroid to function. Based on the comments in the code, there looks to be a very real reason for using iframes for the message content, however I wasn't able to get it to render correctly with the very little knowledge and means of using the C++ API of WebKitGTK, and I can't possibly devote any more time to working out the control flow of the various parts (it's difficult enough stepping through two or more gdb sessions for all the separate processes spawned just to view an email thread).
Maybe this might inspire someone to make a proper fix. :-)
As of WebKitGTK 2.40, enable-frame-flattening is no longer supported, the property does nothing, resulting in all messages in the thread viewer being cut off at around 150px, making all messages longer than 2-3 lines unreadable.
I initially had a look into seeing whether the iframe height could be set programmatically with
webkit_dom_html_iframe_element_set_heightfrom the content of the iframe viawebkit_dom_element_get_scroll_height. However nothing worked at page load time of an email thread, the content scroll height remained at 150 throughout the entireThreadView_on_load_changedpass (because iframes are rendered lazily after the main window thread has finished? Can't think of any other reason why this was observed). The scroll height was only found to update after the thread had finished loading, and triggering a hide/show toggle on each message.Another option would be to use the JavaScriptCore API, and have all the logic to manipulate the height of the iframe in JS. Two possible problems with this: Firstly
enable-javascriptis explicitly disabled in Astroid's WebKit settings; second, potentially all of thewebextensioncode may need to be rewritten in JS - given the huge amount of deprecation warnings when building Astroid, this likely will have to be done in anger at some point in the future anyway.I gave up going down the path of manipulating the iframe after setting
srcdoc, and replaced the iframe with a div instead. CSS has been fixed up and theme version bumped as it would be a breaking change for all existing users that override the theme of the UI.I can't see any concernable difference between before and after when viewing both text/html and text/plain messages, though it is difficult to gauge as my starting point is broken messages.
The
AstroidExtension::reload_imagescode path is the least tested of all the changes. The minimal check I did to see whether the logic is still fine was to send myself an email with a CID attachment, and observe it render correctly after pressing C-i in thread view.As far as this change is concerned, this is one way to fix #720 that suits my use of Astroid. I'm not convinced that it is the correct way to go about it though, as I have no reason to dismiss the rationale for using iframes in the first place - see comments delete by this patch that make reference to style issues, deadlocks and security concerns by not having the content contained to its own iframe.