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

Incorrect page width for plain text messages in Trojita #511

Closed
fturco opened this issue Apr 12, 2017 · 18 comments

Comments

@fturco
Copy link

commented Apr 12, 2017

I noticed a problem with Qtwebkit-6.0 and the Trojita e-mail client.

Plain text messages without any corresponding HTML version are displayed with a page width of only 2 characters.

trojita

This problem happens on both Trojita 0.7 and Trojita git. I suspect that this problem is due to having upgraded Qtwebkit from version 5.7.1 to version 6.0. Trojita developers think the problem may be caused by a regression in Qtwebkit.

My GNU/Linux distribution is Gentoo Linux.

Please also see KDE bug 378684.

@annulen

This comment has been minimized.

Copy link
Member

commented Apr 12, 2017

Is it posible to get HTML test case where it reproduces?

@fturco

This comment has been minimized.

Copy link
Author

commented Apr 12, 2017

I'm afraid I don't understand what you need exactly. Should I provide the HTML source of an e-mail that is affected by this problem perhaps?

@annulen

This comment has been minimized.

Copy link
Member

commented Apr 12, 2017

If Trojita uses QWebView, it has to load some HTML into it. In case of plain text message, they generate it somehow or just load plain text as is, and maybe they also add custom style sheets (e.g. via QWebSettings::setUserStyleSheetUrl or evaluateJavaScript). If we determine what is loaded into QWebView actually, we will be able to reproduce bug without Trojita

Do you think you can try to debug it yourself by following Trojita's code?

@jktjkt

This comment has been minimized.

Copy link

commented Apr 12, 2017

Hi, Trojita upstream here. I suspect that the main reason for failing is that we're trying to undo QtWebKit's built-in QScrollArea by manipulating various layout and margins. Our code has worked with all versions of qtwebkit starting at 4.8.x (or 4.6.x?). Today, Trojita doesn't have any hard dependencies apart from Qt 5.2+ and cmake, so it should be easy to build and try this yourself. The relevant code is at https://github.com/jktjkt/trojita/blob/master/src/Gui/EmbeddedWebView.cpp#L118

@annulen

This comment has been minimized.

Copy link
Member

commented Apr 12, 2017

@jktjkt Thanks for you help! I certainly can try to debug myself, just wanted to offload a bit of work :)

@annulen

This comment has been minimized.

Copy link
Member

commented Apr 12, 2017

@jktjkt BTW, to undo built-in scrolling you can use QWebPage::setPreferredContentsSize, e.g. as it is done in https://github.com/annulen/webkit/blob/qtwebkit-stable/Tools/QtTestBrowser/webview.cpp#L103. Unfortunately in my fork there is a bug with painting backgrounds in this mode, see #472. Also I'm not sure if it can work without QGraphicsView

@annulen annulen added this to the v5.212.0-beta milestone Jun 18, 2017
@elvisangelaccio

This comment has been minimized.

Copy link

commented Jul 8, 2017

Any news on this? Archlinux ships this new version of qt5-webkit in their main repositories, so there are many users affected.

@rapgro

This comment has been minimized.

Copy link

commented Aug 28, 2017

Reproducible for me. This issue landed in Fedora, too.

@darkbasic

This comment has been minimized.

Copy link

commented Sep 27, 2017

Hello everyone. I'm looking for an honest answer: do I need to change my favourite email client? After almost half an year without a fix/workaround I cannot stand it anymore: is a fix ever going to land? Thanks

@jktjkt

This comment has been minimized.

Copy link

commented Sep 29, 2017

@darkbasic , I believe that this depends on @annulen and distributions. We are simply using a component that has regressions; there's nothing that we could change in Trojita :(. Well, we might (and we will, eventually) switch to the QtWebEngine, but that's about all.

@annulen

This comment has been minimized.

Copy link
Member

commented Sep 29, 2017

Actually no, it's because your code is using dirty hack which could be avoided by use of existing API. Anyway, fix exists and will be pushed soon

@jktjkt

This comment has been minimized.

Copy link

commented Sep 29, 2017

@annulen , I'm happy to hear that you've got a patch -- thanks! The solution you pointed out in #511 (comment) won't work for us because Trojita is using the widget based API and not a QGraphicsView. If there's something like that in QWebView, I will be very happy to port our code because it's an ugly workaround, indeed.

@darkbasic

This comment has been minimized.

Copy link

commented Sep 29, 2017

Distributions have nothing to do with this, because the bug is clearly upstream (even in latest released version). They simply cannot freeze the whole qt-webkit to an ancient version because of trojita: other apps need security fixes and the new features.

A good maintainer should have done the following:

  • fix his code if he's doing something wrong
  • otherwise properly fix the broken dependency (if he is able to)
  • in the meantime workaround the issue until a proper fix lands
  • when everything gets fixed contribute proper tests to qt-webkit to avoid that such issues will happen again in the future

Anyway I'm not here to teach anyone what a maintainer should or should not have done (I'm the first one who rarely do the right thing in such regard, so I really cannot point fingers), just please don't tell me that the issue depends on distributions because this is not true.

@annulen I'm glad to hear that there is a fix pending, can you please point me to it so I can apply it to my distribution's qt-webkit?

@kkofler

This comment has been minimized.

Copy link

commented Oct 23, 2017

Ping? You said 24 days ago that a fix exists, can we please have that fix so we can ship it in distributions?

@annulen

This comment has been minimized.

Copy link
Member

commented Oct 23, 2017

@darkbasic

This comment has been minimized.

Copy link

commented Oct 24, 2017

Works flawlessly, thank you very much.

@lupinix

This comment has been minimized.

Copy link

commented Oct 24, 2017

Fix imported in Fedora, fixed qt5-qtwebkit will be available within next hours in updates-testing repository for all supported Fedora releases.

@rapgro

This comment has been minimized.

Copy link

commented Oct 24, 2017

Thank you all. QtWebkit is alive. 👍

@annulen annulen closed this Oct 27, 2017
tcberner added a commit to freebsd/freebsd-ports-kde that referenced this issue Oct 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.