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

+option for display mail format (text/plain | html) (Issue 4456) #748

Closed
wants to merge 2 commits into from
Closed

Conversation

farmboy0
Copy link

This is based on aatdark's implementation.

Please review.

@bomm
Copy link

bomm commented Aug 20, 2015

+1

@Diagoras
Copy link

Looks good to me, for what that's worth.

@Thomas--F
Copy link

Definitely +1

@francwalter
Copy link

I do not understand. Is this pull request merged to the master branch now?
Then where do I find it?
Thanks

@farmboy0
Copy link
Author

The pull request is still open. What has been closed is the old pull request for this feature.

@francwalter
Copy link

@farmboy0: Did you solve this issue? Is this feature working on your fork? Is your fork along the master of k-9? Then I would like to fork your fork :)
Thank

@farmboy0
Copy link
Author

@francwalter I havent rebased my master branch to k-9 master branch since I created this pr.
Youre free to do that yourself. I dont think you will encounter any merge conflicts.
The tests I did with my personal mail were positive but only on the emulator.

@cketti
Copy link
Member

cketti commented Sep 12, 2015

I assume the goal is to preferably display text/plain parts instead of text/html when both are present as alternatives.

Currently the code is in a bit of a transitional state. Previously we went through the message and extracted both a text and an html string to write to the database. The html string was used for display in the user interface. The text string was used as the quoted text when creating replies. When only one of text/plain or text/html is present it is converted to the other format.
Now we save messages in a way that allows us to restore the original structure. To avoid redundancy in the database we no longer extract the text and html strings to write them to the database. Instead we perform this step before displaying the message.
Nowadays, we should only extract an html string because that's what we display in the WebView.

To implement the feature of allowing users to prefer text/plain parts for display we have a couple of options:

  1. Use the old mechanism to extract text and then convert that to HTML for display in the WebView.
  2. Use the old mechanism to extract text and then display that in the WebView using the MIME type text/plain.
  3. Change the extraction code to preferably use text/plain parts when alternatives are available. That part is then converted to HTML and only the html result is returned.

Option 1 has the issue that we might first convert a text/html part to plain text and then back to HTML.
Option 2 isn't great because WebView is kinda bad at displaying text/plain content.
Option 3 is my preferred approach but requires a bit more work.

The code in this pull request implements option 1 only that the "conversion" to HTML is done wrong (i.e. not using HtmlConverter.textToHtml()) and in the wrong place (inside LocalMessageExtractor.buildText() instead of converting the result of LocalMessageExtractor.extractTextAndAttachments()).

@cketti
Copy link
Member

cketti commented Nov 20, 2015

The code to display messages will change a lot because of the PGP/MIME changes. Afterwards it's hopefully easier to properly implement this feature.

Closing this issue for now. I created #906 so we don't forget about this enhancement.

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.

6 participants