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.

Sign up for GitHub

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

Markdown: add support for webkit2gtk #746

Merged
merged 1 commit into from
Jun 3, 2018
Merged

Markdown: add support for webkit2gtk #746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions build/markdown.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,13 @@ AC_DEFUN([GP_CHECK_MARKDOWN],
GTK_VERSION=2.16
WEBKIT_VERSION=1.1.13

GP_CHECK_GTK3([webkit_package=webkitgtk-3.0],
GP_CHECK_GTK3([webkit_package=webkit2gtk-4.0],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hyperair I wonder if we can cascade from webkit-1.0 to webkit-3.0 to webkitgtk-4.0 to try and support all versions?

Copy link
Member Author

@codebrainz codebrainz May 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not tested, but I mean like this:

GP_CHECK_GTK3([webkit_package=webkit2gtk-4.0], [
  GP_CHECK_GTK3([webkit_package=webkitgtk-3.0], [
    webkit_package=webkit-1.0
  ])
])
...
AM_CONDITIONAL([MARKDOWN_WEBKIT2], [test "x$webkit_package" = "xwebkit2gtk-4.0"])
...

@b4n @sardemff7 @dmaphy @evgeni @hyperair care to comment?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should work, but you better not add fallback code for unsupported versions, distributions with older packages will use older plugin too. (And people really must stop thinking everything will always work for them without patching on older systems, do not but the maintenance burden on you.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the versions listed are "supported" in that they're known to build and work for the plugin, but not that they are "supported" upstream, obviously. IMO, if it's a simple matter of a few ugly #ifdef it's worthwhile even if just to allow people on slightly older distros to compile the latest plugin release tarball.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be useful -- on Ubuntu's side, there's still one remaining release (trusty) that has webkitgtk-3.0 but not webkit2gtk-4.0.

[webkit_package=webkit-1.0])

GP_CHECK_PLUGIN_DEPS([markdown], [MARKDOWN],
[$GP_GTK_PACKAGE >= ${GTK_VERSION}
$webkit_package >= ${WEBKIT_VERSION}
gthread-2.0])
AM_CONDITIONAL([MARKDOWN_WEBKIT2], [test "$webkit_package" = webkit2gtk-4.0])

GP_COMMIT_PLUGIN_STATUS([Markdown])

Expand Down
4 changes: 4 additions & 0 deletions markdown/src/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,8 @@ markdown_la_CFLAGS += $(LIBMARKDOWN_CFLAGS)
markdown_la_LIBADD += $(LIBMARKDOWN_LIBS)
endif

if MARKDOWN_WEBKIT2
markdown_la_CFLAGS += -DMARKDOWN_WEBKIT2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not an AC_DEFINE in markdown.m4 instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it does seem better.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AC_DEFINE affects everything, so plugins that don't care about this variable will get it too. I originally wanted to just have a WEBKIT2 macro defined only for markdown based on the MARKDOWN_WEBKIT2 conditional, but @codebrainz changed that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't clash because I added the namespace prefix, so it doesn't seem like an issue.

endif

include $(top_srcdir)/build/cppcheck.mk
29 changes: 28 additions & 1 deletion markdown/src/viewer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@
#include "config.h"
#include <string.h>
#include <gtk/gtk.h>
#include <webkit/webkitwebview.h>
#ifdef MARKDOWN_WEBKIT2
# include <webkit2/webkit2.h>
#else
# include <webkit/webkitwebview.h>
#endif
#include <geanyplugin.h>
#ifndef FULL_PRICE
# include <mkdio.h>
Expand Down Expand Up @@ -296,6 +300,18 @@ pop_scroll_pos(MarkdownViewer *self)
return popped;
}

#ifdef MARKDOWN_WEBKIT2
static void
on_webview_load_changed(MarkdownViewer *self,
WebKitLoadEvent load_event,
WebKitWebView *web_view)
{
/* When the webkit is done loading, reset the scroll position. */
if (load_event == WEBKIT_LOAD_FINISHED) {
pop_scroll_pos(self);
}
}
#else
static void
on_webview_load_status_notify(WebKitWebView *view, GParamSpec *pspec,
MarkdownViewer *self)
Expand All @@ -309,6 +325,7 @@ on_webview_load_status_notify(WebKitWebView *view, GParamSpec *pspec,
pop_scroll_pos(self);
}
}
#endif

gchar *
markdown_viewer_get_html(MarkdownViewer *self)
Expand Down Expand Up @@ -388,13 +405,23 @@ markdown_viewer_update_view(MarkdownViewer *self)
/* Connect a signal handler (only needed once) to restore the scroll
* position once the webview is reloaded. */
if (self->priv->load_handle == 0) {
#ifdef MARKDOWN_WEBKIT2
self->priv->load_handle =
g_signal_connect_swapped(WEBKIT_WEB_VIEW(self), "load-changed",
G_CALLBACK(on_webview_load_changed), self);
#else
self->priv->load_handle =
g_signal_connect_swapped(WEBKIT_WEB_VIEW(self), "notify::load-status",
G_CALLBACK(on_webview_load_status_notify), self);
#endif
}

#ifdef MARKDOWN_WEBKIT2
webkit_web_view_load_html(WEBKIT_WEB_VIEW(self), html, base_uri);
#else
webkit_web_view_load_string(WEBKIT_WEB_VIEW(self), html, "text/html",
self->priv->enc, base_uri);
#endif

g_free(base_uri);
g_free(html);
Expand Down
6 changes: 5 additions & 1 deletion markdown/src/viewer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@
#define MARKDOWN_VIEWER_H 1

#include <gtk/gtk.h>
#include <webkit/webkitwebview.h>
#ifdef MARKDOWN_WEBKIT2
# include <webkit2/webkit2.h>
#else
# include <webkit/webkitwebview.h>
#endif

G_BEGIN_DECLS

Expand Down