MGR: support wxWidgets without webview #2093

jengelh · 2017-09-08T00:00:46Z

If wxWidgets is built without the webview widget, make do with a
plain multiline text control.

--
openSUSE plans on removing webkitgtk. wxWidgets-3.0.X has it as an optional dependency but turning it off makes programs like boinc fail to build. Solve that in short order - the patch is not necessarily beautiful, though the resulting UI seems to do well, as most notices have very little markup (mostly just <a>..</a>).

AenBleidd · 2017-09-08T00:15:26Z

I'm sorry, my previous comment was related to another stuff. I've deleted it. Best regards, Vitalii Sent via Android 8 сент. 2017 г. 3:10 пользователь "Vitalii Koshura" < lestat.de.lionkur@gmail.com> написал:

…

There is no possibility to show text aligned to two columns using plain text. Also even if I'll calculate the initial position of the text it will be broken when user change to size of the window. It is possible to recalculate text position on every window size change but this will lead to UI glitches. Anyway I'll try to do this. Best regards, Vitalii Sent via Android 8 сент. 2017 г. 3:00 пользователь "jengelh" ***@***.***> написал: > If wxWidgets is built without the webview widget, make do with a > plain multiline text control. > > -- > openSUSE plans on removing webkitgtk. wxWidgets-3.0.X has it as an > optional dependency but turning it off makes programs like boinc fail to > build. Solve that in short order - the patch is not necessarily beautiful, > though the resulting UI seems to do well, as most notices have very little > markup (mostly just <a>..</a>). > ------------------------------ > You can view, comment on, or merge this pull request online at: > > #2093 > Commit Summary > > - MGR: support wxWidgets without webview > > File Changes > > - *M* clientgui/NoticeListCtrl.cpp > <https://github.com/BOINC/boinc/pull/2093/files#diff-0> (47) > - *M* clientgui/NoticeListCtrl.h > <https://github.com/BOINC/boinc/pull/2093/files#diff-1> (6) > > Patch Links: > > - https://github.com/BOINC/boinc/pull/2093.patch > - https://github.com/BOINC/boinc/pull/2093.diff > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#2093>, or mute the thread > <https://github.com/notifications/unsubscribe-auth/ADFZoatyroSSTpK54sBzt-g4yHGYn5Hfks5sgIOwgaJpZM4PQiMD> > . >

CharlieFenton · 2017-09-08T00:37:34Z

These patches sometimes use compile time conditionals (#if #else #endif) and sometimes use run-time conditionals (if else). Although the compiler should optimize out the run-time conditionals, I think it would make cleaner code to use only compile-time conditionals throughout. Also, this does not take into account the fact that notices can include images, though this capability is not used often.

ChristianBeer · 2017-09-08T08:16:36Z

The Debian distribution also wants to drop support for wxWebView and we discussed possible solutions. One that sounded the most promising is to replace wxWebView with wxHtmlWindow which does not require webkitgtk but still provides similar features.

AenBleidd · 2017-09-08T08:24:58Z

@ChristianBeer, as I remember wxHTMLWindow doesn't support styles (or has very limited support). So I believe it can't fully replace wxWebView. Best regards, Vitalii Sent via Android 8 сент. 2017 г. 11:16 пользователь "Christian Beer" < notifications@github.com> написал:

…

The Debian distribution also wants to drop support for wxWebView and we discussed possible solutions. One that sounded the most promising is to replace wxWebView with wxHtmlWindow which does not require webkitgtk but still provides similar features. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2093 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADFZoRYzjNo0q8tP-cDkMXn-H3i7PwMnks5sgPflgaJpZM4PQiMD> .

ChristianBeer · 2017-09-08T08:54:25Z

In the places we use wxWebView we don't use a lot of styles so it is a better alternative than wxTextCtrl where we have no style support at all.

AenBleidd · 2017-09-08T09:12:12Z

In my case in PropertiesWindows wxHTMLWindow is not enough. I plan to add necessary functionality to this standard control. Best regards, Vitalii Koshura 2017-09-08 11:54 GMT+03:00 Christian Beer <notifications@github.com>:

…

In the places we use wxWebView we don't use a lot of styles so it is a better alternative than wxTextCtrl where we have no style support at all. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2093 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADFZodBEmZp-xwaYlHrSQvqrmCbgk_APks5sgQDCgaJpZM4PQiMD> .

jengelh · 2017-09-08T12:14:49Z

Updated to have it use wxHtmlWindow. Good suggestion.

JuhaSointusalo · 2017-09-08T13:41:54Z

I think this is a bad idea. One of the reasons Manager was changed to use wxWebView was so that it can show notices that embed videos. And by being backed by a real web browser engine wxWwebView can do much more than just that. It handles SVGs, or you can use Javasript and have dynamic content or anything you could have in a web page and that is pretty cool.

Having rich content in apps is normal these days and having the content in the app likely results in larger percentage of the audience actually seeing the content. You lose a part of the audience at every click they have to do.

This will also make Manager's features differ between platforms. It's never fun to spend a couple of hours trying to help someone only to find out that the version of BOINC they have lacks a feature you have on your BOINC.

Before anyone objects that nobody uses videos in notices. Now that everyone is in Paris how about promoting the feature a bit. Seti@home for example have had staff video interviews but the videos have not been embedded in the notices.

openSUSE is presumably removing the old unsupported totally insecure version of webkitgtk but they also have the newer supported version packaged in webkit2gtk3. Rather than this patch please work with wxWidgets' packager to build wxWidgets with the newer WebKitGTK+.

JuhaSointusalo · 2017-09-08T13:49:58Z

clientgui/AsyncRPC.cpp

@@ -20,7 +20,7 @@
 #endif

 #if !(defined(_WIN32) || (defined(__WXMAC__) && (MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_4)))
-#include <xlocale.h>
+#include <locale.h>


This is wrong. See PR #2092.

JuhaSointusalo · 2017-09-08T13:52:02Z

clientgui/NoticeListCtrl.h

@@ -45,8 +45,12 @@ class CNoticeListCtrl: public wxWindow

 ////@begin CNoticeListCtrl event handler declarations

+#if defined(wxUSE_WEBVIEW) && wxUSE_WEBVIEW


Elsewhere you use simple #if wxUSE_WEBVIEW.

JuhaSointusalo

If this ends up being merged some changes are needed.

jengelh · 2017-09-08T14:44:26Z

wxWidgets 3.0.3 does not appear to support webkit2. It only checks for webkit-1.0.pc/webkitgtk-3.0.pc.

If wxWidgets is built without support for the webview widget, make do with a wxHtmlWindow instead.

JuhaSointusalo · 2017-09-08T15:02:20Z

You'd need to backport the support. See the discussion in Debian bugtracker.

jengelh · 2017-09-08T17:36:18Z

backport-for-boinc-7.6.x.diff.txt

JuhaSointusalo · 2017-09-08T18:17:10Z

Sorry. I meant, you'd need to backport the webkit2gtk support of the later version of wxWidgets to wxWidgets version 3.0.3 and then have the wxWidgets 3.0.3+webkit2gtk packaged for openSUSE. From the Debian thread it looks like it's only one commit that's needed.

What kind of timetable openSUSE has for dropping the old webkitgtk and releasing a new distro version?

davidpanderson · 2017-09-08T18:59:39Z

I'd like to keep the ability to show video in notices if possible
(eventually projects will use this feature, I hope).

jengelh · 2017-09-08T19:46:52Z

What kind of timetable openSUSE has for dropping the old webkitgtk

webkit I don't know yet (other maintainer), but the WebView-less wxWidgets will likely find its way onto mirrors next week already. There wasn't a whole lot of packages — two — that unconditionally used that class to begin with.

ChristianBeer · 2017-09-09T08:28:46Z

The change from wxWebView to wxHTMLWindow was always meant as an intermediate measure until the Debian/OpenSuse package maintainers can get the new webkitgtk packages up and running. The alternative is that BOINC will be removed from Debian and openSuse repositories and I don't think we want this to happen. So we will loose some functionality until the webkitgtk update is finished but it will not be lost forever.

JuhaSointusalo · 2017-09-09T17:44:31Z

If at some point it comes to choosing either reduced functionality or Manager being dropped from repositories I can live with reduced functionality. On Debian it looks like they are not yet (as in today or tomorrow) removing webkitgtk.

I tried to find more information about openSUSE's situation but didn't find anything (maybe I didn't look at the right place). What I did find out is that @jengelh is wxWidgets maintainer there so he is in perfect position to sort this out on their side.

I'm being difficult here in the hopes that it will put pressure on the distros to move to a supported WebKitGTK. But I have a feeling that I'm losing the argument here. With the way the code is right now, if someone builds the Manager with wxWidgets that lacks WebView, the code silently switches to reduced functionality. I'd prefer if reduced functionality is something one has to opt-in. If one doesn't opt-in then break the build.

JuhaSointusalo

wx/webview.h and wx/webviewfshandler.h are included in clientgui/stdwx.h. I would imagine that if wxWidgets is built without WebView then these headers are not available.

jengelh · 2017-09-12T18:40:08Z

I would imagine that if wxWidgets is built without WebView then these headers are not available.

Given that webview.h has a big #if wxUSE_WEBVIEW inside it, that suggests it is always available (though of course "empty" for all practical purposes).

JuhaSointusalo · 2017-09-12T18:58:21Z

I didn't expect that but you are right. Never mind then.

ojwb · 2017-09-27T21:15:00Z

From the Debian thread it looks like it's only one commit that's needed.

However, it's not just a matter of backporting a single commit or we'd have done that long ago - the newer webkitgtk also requires a switch from GTK+ 2 to GTK+ 3, and wxWidgets' GTK+ 3 support isn't yet as solid as its GTK+ 2 support - even trying the samples that come with wxWidgets turned up several bugs under GTK+ 3. The wxWidgets developers are pretty responsive to fixing these, but a switch to GTK+ 3 is going to need a lot of time and testing. If it comes down to having to choose, I'm afraid the short term decision for Debian would be to temporarily break boinc in testing/unstable rather than rush into switching to GTK+ 3 and break an unknown number of other packages which use wxWidgets.

On Debian it looks like they are not yet (as in today or tomorrow) removing webkitgtk.

However, it is likely a matter of days now - it's only boinc and empathy that are left, and I was told empathy was "several days away" at the start of this week.

What's the status of the patch here?

CharlieFenton · 2017-09-27T21:37:24Z

I don't know much about Linux distros, but I wonder if it would be practical for someone to build the webview.so library from wxWidgets and make the library available in GIT for static linking with BOINC Manager on Linux?

ojwb · 2017-09-27T23:22:58Z

No, that wouldn't help at all.

JuhaSointusalo · 2017-09-28T21:04:38Z

@ojwb Thanks for the update on Debian status. Lifting this comment from Debian bug 790222

As things stand, the plan is probably that there's no webview for buster.

That makes it pretty clear what the current status is.

I tested this PR on both Windows and Linux. I don't have wxWidgets that's really without Webview so I edited setup.h to disable it on Linux.

The good news is that on Windows Manager still used wxWebView and on Linux it used wxHTMLWindow. Bad news is that everything else sucks.

On Linux,

clicking any link now opens the link both inside Manager and in proper web browser
images over HTTPS are not supported
SVG is not supported (although, on Windows IE engine is used which doesn't support SVG either)
video isn't supported

Video support totally sucks in WebView enabled Manager btw. YouTube doesn't support Flash based player any more, instead you get some kind of an Flash thingy with "Flash-embedded videos are no longer supported" text and a button "Watch on YouTube" but clicking the button does nothing on Linux or Windows.

And YouTube's new iframe based player doesn't work on Linux or Windows either. In Manager it's just empty space and on top of that on Linux it automatically opens the video (not the news post) in browser tab. On Windows you need right click the Notices tab and select Refresh to have the video open automatically.

And if that's not bad enough, HTML5 video doesn't work on Windows because the IE engine doesn't support it.

:(

JuhaSointusalo · 2017-09-28T21:09:07Z

@ojwb I forgot to add. It's not just this PR but also PR #2104 that's needed before BOINC doesn't require WebView.

AenBleidd · 2017-09-28T21:12:08Z

@JuhaSointusalo, I work on PR #2104 <#2104>. I hope I can finish it on this week. Thanks Best regards, Vitalii Koshura 2017-09-29 0:09 GMT+03:00 Juha Sointusalo <notifications@github.com>:

…

@ojwb <https://github.com/ojwb> I forgot to add. It's not just this PR but also PR #2104 <#2104> that's needed before BOINC doesn't require WebView. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2093 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADFZobVAFQt-9_hj2U1-wr8EgjEv5hFZks5snAr0gaJpZM4PQiMD> .

JuhaSointusalo · 2017-09-29T18:48:46Z

clientgui/NoticeListCtrl.cpp

+		event.Skip();
+		return;
+	}
+	event.Skip(); // Tell element not to follow link


Bug. Contrary to the comment, it makes wxHTMLWindow open the link inside Manager.

JuhaSointusalo · 2017-09-29T18:55:43Z

Images over HTTPS don't work because wxWidgets doesn't support HTTPS. That's kind of bad considering the push to HTTPS everywhere.

I suppose HTTPS URLs could be rewritten to HTTP and hoping that the web server doesn't redirect all HTTP to HTTPS. Or an HTTPS handler could be added but that's lot of work. Maybe we should just accept that the next Debian, Ubuntu and OpenSUSE releases won't always display images in notices.

This is a combination of contributions by Jan Engelhardt (#2093) and Olly Betts (https://anonscm.debian.org/cgit/pkg-boinc/boinc.git/commit/?id=60f4cd232522db0750b2dff56bd327dc44a51534) to make the Manager work without webview support. This is mainly needed for Linux distributions that are migrating to a newer webkitgtk library.

ojwb · 2017-11-10T01:08:01Z

Lifting this comment from Debian bug 790222

As things stand, the plan is probably that there's no webview for buster.

That makes it pretty clear what the current status is.

You've lifted that without context - in full:

> what is the plan to fix it BTW? moving to the new webkitgtk and transition to
> gtk3?

I think that's the only way to get webview back, but we need people to
actually do the work to make that happen.  I'm not realistically able to
commit to doing a transition unaided at this point - initial
investigations showed that it's likely to shake out a significant number
of bugs in both wx's GTK+ 3 support and in apps themselves.  It's not
just a case of uploading and binNMUing all rdeps.

As things stand, the plan is probably that there's no webview for buster.

The tl;dr version is more like "help wanted".

This is a combination of contributions by Jan Engelhardt (#2093) and Olly Betts (https://anonscm.debian.org/cgit/pkg-boinc/boinc.git/commit/?id=60f4cd232522db0750b2dff56bd327dc44a51534) to make the Manager work without webview support. This is mainly needed for Linux distributions that are migrating to a newer webkitgtk library.

AenBleidd · 2018-02-05T11:46:25Z

Can be closed as unnecessary after merging #2190

AenBleidd · 2018-02-05T18:56:15Z

Necessary changes are already merged into master via #2190.
I'm closing this pull request now.
If you have any objections feel free to reopen it.

ojwb · 2018-03-26T22:39:57Z

FYI, there's now a GTK3 build of wxwidgets3.0 in Debian unstable and testing, which includes the webview library (thanks to Scott Talbert stepping up to make this happen).

JuhaSointusalo reviewed Sep 8, 2017

View reviewed changes

JuhaSointusalo suggested changes Sep 8, 2017

View reviewed changes

MGR: support wxWidgets without webview

1039953

If wxWidgets is built without support for the webview widget, make do with a wxHtmlWindow instead.

ChristianBeer mentioned this pull request Sep 12, 2017

[WIP] Manager: Use wxHtmlWindow in DlgItemProperties. #2104

Closed

JuhaSointusalo suggested changes Sep 12, 2017

View reviewed changes

JuhaSointusalo reviewed Sep 29, 2017

View reviewed changes

ChristianBeer mentioned this pull request Oct 26, 2017

make webview support optional #2190

Merged

AenBleidd closed this Feb 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MGR: support wxWidgets without webview #2093

MGR: support wxWidgets without webview #2093

jengelh commented Sep 8, 2017

AenBleidd commented Sep 8, 2017 via email

CharlieFenton commented Sep 8, 2017 via email

ChristianBeer commented Sep 8, 2017

AenBleidd commented Sep 8, 2017 via email

ChristianBeer commented Sep 8, 2017

AenBleidd commented Sep 8, 2017 via email

jengelh commented Sep 8, 2017

JuhaSointusalo commented Sep 8, 2017

JuhaSointusalo Sep 8, 2017

JuhaSointusalo Sep 8, 2017

JuhaSointusalo left a comment

jengelh commented Sep 8, 2017

JuhaSointusalo commented Sep 8, 2017

jengelh commented Sep 8, 2017

JuhaSointusalo commented Sep 8, 2017

davidpanderson commented Sep 8, 2017

jengelh commented Sep 8, 2017

ChristianBeer commented Sep 9, 2017

JuhaSointusalo commented Sep 9, 2017

JuhaSointusalo left a comment

jengelh commented Sep 12, 2017

JuhaSointusalo commented Sep 12, 2017 via email

ojwb commented Sep 27, 2017

CharlieFenton commented Sep 27, 2017

ojwb commented Sep 27, 2017

JuhaSointusalo commented Sep 28, 2017

JuhaSointusalo commented Sep 28, 2017

AenBleidd commented Sep 28, 2017 via email

JuhaSointusalo Sep 29, 2017

JuhaSointusalo commented Sep 29, 2017

ojwb commented Nov 10, 2017

AenBleidd commented Feb 5, 2018

AenBleidd commented Feb 5, 2018

ojwb commented Mar 26, 2018

		@@ -45,8 +45,12 @@ class CNoticeListCtrl: public wxWindow

		////@begin CNoticeListCtrl event handler declarations

		#if defined(wxUSE_WEBVIEW) && wxUSE_WEBVIEW

MGR: support wxWidgets without webview #2093

MGR: support wxWidgets without webview #2093

Conversation

jengelh commented Sep 8, 2017

AenBleidd commented Sep 8, 2017 via email

CharlieFenton commented Sep 8, 2017 via email

ChristianBeer commented Sep 8, 2017

AenBleidd commented Sep 8, 2017 via email

ChristianBeer commented Sep 8, 2017

AenBleidd commented Sep 8, 2017 via email

jengelh commented Sep 8, 2017

JuhaSointusalo commented Sep 8, 2017

JuhaSointusalo Sep 8, 2017

Choose a reason for hiding this comment

JuhaSointusalo Sep 8, 2017

Choose a reason for hiding this comment

JuhaSointusalo left a comment

Choose a reason for hiding this comment

jengelh commented Sep 8, 2017

JuhaSointusalo commented Sep 8, 2017

jengelh commented Sep 8, 2017

JuhaSointusalo commented Sep 8, 2017

davidpanderson commented Sep 8, 2017

jengelh commented Sep 8, 2017

ChristianBeer commented Sep 9, 2017

JuhaSointusalo commented Sep 9, 2017

JuhaSointusalo left a comment

Choose a reason for hiding this comment

jengelh commented Sep 12, 2017

JuhaSointusalo commented Sep 12, 2017 via email

ojwb commented Sep 27, 2017

CharlieFenton commented Sep 27, 2017

ojwb commented Sep 27, 2017

JuhaSointusalo commented Sep 28, 2017

JuhaSointusalo commented Sep 28, 2017

AenBleidd commented Sep 28, 2017 via email

JuhaSointusalo Sep 29, 2017

Choose a reason for hiding this comment

JuhaSointusalo commented Sep 29, 2017

ojwb commented Nov 10, 2017

AenBleidd commented Feb 5, 2018

AenBleidd commented Feb 5, 2018

ojwb commented Mar 26, 2018