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

Optionally display message body without HTML #3891

Conversation

dhebbeker
Copy link
Contributor

@dhebbeker dhebbeker commented Jan 27, 2019

This PR adds a setting which allows the user to disable HTML formatting of the message body. It does so by adding an option in the general view settings.

Changes

A new setting has been defined and is stored persistently (using K9::loadPrefs() and K9::save()) with the key "messageViewFixedWidthFont". This setting is presented to the user as the CheckBox with the key "messageview_without_html". The setting is loaded at runtime into K9::messageViewWithoutHtml.

In order to display plain text a TextView element message_plain_text has been added to message_container.xml. An additional parameter has been added to MessageViewInfo::MessageViewInfo() in order to pass the plain text part of the message to MessageContainerView::displayMessageViewContainer(). Depending on the state of the setting this method does either display the HTML part in the WebView (as before) or populates the TextView with the plain text. The other element is hidden from the user.

Related open issues and PRs

Issue #906 requests

the user to display text/plain parts of emails by default.

This PR does allow the user to display an unformatted message without a WebView element. The difference of this PR to that issue the user is, that in case the message does not contain a text/plain part, the HTML part is converted to plain text (this is an already established behavior defined in MessageViewInfoExtractor::buildText()) and displayed in a TextView. The precise solution for #906 would be to display no text in this case. As this behavior is expected to be rare, this PR essentially does fix #906.

This PR is related to issue #1397 which does request a feature to

Toggle view between plain Text and HTML

In contrast to that, this PR does not allow to toggle between the text/plain or text/html part, but to switch off HTML formatting. The difference is, that with this PR a message is always shown, even if no HTML part has been sent. This is a rather formal difference. A thorough solution for #1397 would be to offer the plain or HTML part only if they were part of the message.

This PR is based on PR #2334. It has been updated with the current master. The user texts and design have been slightly adjusted in order to resemble to the current design using WebView.

Design rationale

Users in #906 and #1397 motivated these issues with security concerns displaying HTML. Thus WebView is not used in case the user disables HTML formatted messages. Instead a TextView element has been added.

In order to render both cases (HTML or plain text) with one method (MessageContainerView::displayMessageViewContainer()), method signatures had to be extended by a String parameter containing the plain text.

Checks

Please ensure that your pull request meets the following requirements - thanks !

  • Follows our existing codestyle.
    ✔️ Coding guidelines have been applied with the help of k-9/tools/android-studio/settings.jar.
  • Does not break any unit tests.
    ✔️ Unit tests have been executed:
./gradlew testDebug
BUILD SUCCESSFUL in 1m 57s
336 actionable tasks: 78 executed, 258 up-to-date

spchand86 and others added 16 commits March 4, 2017 04:54
Many files were renamed within 'k9mail/master', which may lead to difficulties in tracking the differences.

This merge may be used to update the pull request at thunderbird#2334.
In order to adjust the layout to the one of HTML (WebView) text.
Using `tools/android-studio/settings.jar`

(cherry picked from commit d5877ef576745e30a0d22104009aed882961b8cf)
Renaming variables in the context of HTML / plain text messages.

Rewording of checkbox texts.
The HTML related rendering activities only need to be executed in case HTML is shown.
…wWithoutHtml".

This way it is consistently throughout the code base.
The version integral needs to be incremented according to comment in GlobalSettings.java:47.
This new version number is applied to the newly added setting "messageViewWithoutHtml".

This commit is prone to merge conflicts. To resolve such a conflict increment the version from their branch and apply new number on all places changed within this commit.
Merge remote-tracking branch 'k9mail/master' into feature/toggle-between-plain-text-html/v2.

This facilitates merging future changes.

Tested that the merge result still works.
@cketti
Copy link
Member

cketti commented Feb 12, 2019

Thanks for spending the time to implement a new feature. Unfortunately, this is not getting us closer to where I want the app to go.

As stated here I don't want to add a different way to display messages (i.e. no TextView). What I want is an option to switch between the alternative parts of an email. And when we have that it also makes sense to configure which one should be the default.

@cketti cketti closed this Feb 12, 2019
@dhebbeker
Copy link
Contributor Author

Thank you for assessing this pull request @cketti. Your feedback helps me to implement a better fitting feature.

As stated here I don't want to add a different way to display messages (i.e. no TextView).

Based on the commits added to this pull request so far, this should not be much effort to implement.

What I want is an option to switch between the alternative parts of an email. And when we have that it also makes sense to configure which one should be the default.

Do you suggest to introduce the changes in at least two stages: First merging changes which enable the user to switch between the alternative parts from within an individual message; and afterwards add a setting to the global display preferences to set the default alternative to be shown. The first part would fix #1397, the latter fix #906. Did I get it right?

I would like to contribute to this implementation such as it fits best to K-9 Mail. I am grateful for feedback on my assumptions, especially by the authors of the respective issues (@cketti, @webratte)!

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.

Add option for default display format
3 participants