Patch for xembed (text corrected)

[podarcis]

Hi all,

Here is a patch to make embedding dillo into an X-window work.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff -r 9bad41ea1dde src/xembed.cc
--- a/src/xembed.cc Wed Dec 05 13:03:04 2012 +0100
+++ b/src/xembed.cc Sun Dec 09 10:12:36 2012 +0100
// TODO: Implement more XEMBED support;

-void Xembed::show() {
+void Xembed::create() {
createInternal(xid);
setXembedInfo(1);
Fl::event_dispatch(event_handler);
diff -r 9bad41ea1dde src/xembed.hh
--- a/src/xembed.hh Wed Dec 05 13:03:04 2012 +0100
+++ b/src/xembed.hh Sun Dec 09 10:12:36 2012 +0100
@@ -14,10 +14,11 @@

public:
Xembed(uint32_t xid, int _w, int _h) : Fl_Window(_w, _h) {
- this->xid = xid;
+ this->xid = xid; create();
};
- void show();
+ void create();
int handle(int event);
+ void resize(int x, int y, int w, int h);
};

#endif
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

tested with this FLTK program:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#include <FL/Fl.H>
#include <FL/Fl_Window.H>
#include <FL/x.H>
#include <cstdio>
#include <iostream>
#include <sstream>
#include <string>

using namespace std;


int main( int argc_, char *argv_[] )
{
Fl_Window dw( 800, 600, "dillo embedded" );
dw.resizable( &dw );
dw.show();
cout << "dw.xid = " << (unsigned long)fl_xid(&dw) << endl;
ostringstream cmd;
cmd << "dillo -f -g " <<
dw.w() << "x" << dw.h() << " --xid " <<
(unsigned long)fl_xid(&dw);
if ( argc_ > 1 )
cmd << " " << argv_[1];
cmd << " &";
cout << "system '" << cmd.str() << "'" << endl;
system( cmd.str().c_str() );
return Fl::run();
}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

What does NOT work is automatic resizing of the embedded dillo window,
when the size is changed. This would be nice to have.

I do not know enough of the embedding stuff, but I wonder if the static
int event_handler() ever gets called with type ClientMessage - at least
I could not make it get there..

podarcis

[Johannes Hofmann]

Hi,

I test xembed support with the claws mail dillo plugin[1] and
tabbed[2]. My guess is that the embedder needs to send some X events
to the embedded program (dillo in this case) to make it work
properly.
Even with your patch dillo doesn't resize properly for me when I
change the embedding window.
I would look into the tabbed source code[2], which is pretty
minimal.

Cheers,
Johannes

[1] http://www.claws-mail.org/plugin.php?plugin=dillo
[2] http://tools.suckless.org/tabbed/

Post by podarcis
Hi all,
Here is a patch to make embedding dillo into an X-window work.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff -r 9bad41ea1dde src/xembed.cc
--- a/src/xembed.cc Wed Dec 05 13:03:04 2012 +0100
+++ b/src/xembed.cc Sun Dec 09 10:12:36 2012 +0100
// TODO: Implement more XEMBED support;
-void Xembed::show() {
+void Xembed::create() {
createInternal(xid);
setXembedInfo(1);
Fl::event_dispatch(event_handler);
diff -r 9bad41ea1dde src/xembed.hh
--- a/src/xembed.hh Wed Dec 05 13:03:04 2012 +0100
+++ b/src/xembed.hh Sun Dec 09 10:12:36 2012 +0100
@@ -14,10 +14,11 @@
Xembed(uint32_t xid, int _w, int _h) : Fl_Window(_w, _h) {
- this->xid = xid;
+ this->xid = xid; create();
};
- void show();
+ void create();
int handle(int event);
+ void resize(int x, int y, int w, int h);
};
#endif
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#include <FL/Fl.H>
#include <FL/Fl_Window.H>
#include <FL/x.H>
#include <cstdio>
#include <iostream>
#include <sstream>
#include <string>
using namespace std;
int main( int argc_, char *argv_[] )
{
Fl_Window dw( 800, 600, "dillo embedded" );
dw.resizable( &dw );
dw.show();
cout << "dw.xid = " << (unsigned long)fl_xid(&dw) << endl;
ostringstream cmd;
cmd << "dillo -f -g " <<
dw.w() << "x" << dw.h() << " --xid " <<
(unsigned long)fl_xid(&dw);
if ( argc_ > 1 )
cmd << " " << argv_[1];
cmd << " &";
cout << "system '" << cmd.str() << "'" << endl;
system( cmd.str().c_str() );
return Fl::run();
}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
What does NOT work is automatic resizing of the embedded dillo window,
when the size is changed. This would be nice to have.
I do not know enough of the embedding stuff, but I wonder if the static
int event_handler() ever gets called with type ClientMessage - at least
I could not make it get there..
podarcis
_______________________________________________
Dillo-dev mailing list
http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

[podarcis]

Hi,

here is an updated version making resize work.
It still works with tabbed too.
Note that I have had no knowledge of X or xembed at all, so all changes
come from experimenting...
I removed large parts of the existing code, because they did nothing in
my environment.

Please have a look at it.

Cheers,
podarcis

-----
diff -r 52633c6c3fc6 src/xembed.cc
--- a/src/xembed.cc Wed Dec 12 19:40:33 2012 +0000
+++ b/src/xembed.cc Sat Dec 15 11:49:23 2012 +0100
@@ -21,93 +21,49 @@

#ifdef X_PROTOCOL

-typedef enum {
- XEMBED_EMBEDDED_NOTIFY = 0,
- XEMBED_WINDOW_ACTIVATE = 1,
- XEMBED_WINDOW_DEACTIVATE = 2,
- XEMBED_REQUEST_FOCUS = 3,
- XEMBED_FOCUS_IN = 4,
- XEMBED_FOCUS_OUT = 5,
- XEMBED_FOCUS_NEXT = 6,
- XEMBED_FOCUS_PREV = 7,
- XEMBED_GRAB_KEY = 8,
- XEMBED_UNGRAB_KEY = 9,
- XEMBED_MODALITY_ON = 10,
- XEMBED_MODALITY_OFF = 11,
-} XEmbedMessageType;
-
-void
-Xembed::setXembedInfo(unsigned long flags)
-{
- unsigned long buffer[2];
-
- Atom xembed_info_atom = XInternAtom (fl_display, "_XEMBED_INFO", false);
-
- buffer[0] = 1;
- buffer[1] = flags;
-
- XChangeProperty (fl_display,
- xid,
- xembed_info_atom, xembed_info_atom, 32,
- PropModeReplace,
- (unsigned char *)buffer, 2);
+void Xembed::update_size() {
+ XWindowAttributes winAttributes;
+ XGetWindowAttributes( fl_display, xid, &winAttributes );
+ int width = winAttributes.width;
+ int height = winAttributes.height;
+ update_size(width, height);
}

-void
-Xembed::sendXembedEvent(uint32_t message) {
- XClientMessageEvent xclient;
-
- memset (&xclient, 0, sizeof (xclient));
- xclient.window = xid;
- xclient.type = ClientMessage;
- xclient.message_type = XInternAtom (fl_display, "_XEMBED", false);
- xclient.format = 32;
- xclient.data.l[0] = fl_event_time;
- xclient.data.l[1] = message;
-
- XSendEvent(fl_display, xid, False, NoEventMask, (XEvent *)&xclient);
- XSync(fl_display, False);
+void Xembed::update_size(int width, int height) {
+ if (w() != width || h() != height)
+ {
+ resize(x(), y(), width, height);
+ redraw();
+ }
}

-int
-Xembed::handle(int e) {
- if (e == FL_PUSH)
- sendXembedEvent(XEMBED_REQUEST_FOCUS);
-
- return Fl_Window::handle(e);
-}
-
+static Xembed *xembed = 0;
static int event_handler(int e, Fl_Window *w) {
- Atom xembed_atom = XInternAtom (fl_display, "_XEMBED", false);
-
- if (fl_xevent->type == ClientMessage) {
- if (fl_xevent->xclient.message_type == xembed_atom) {
- long message = fl_xevent->xclient.data.l[1];
-
- switch (message) {
- case XEMBED_WINDOW_ACTIVATE:
- // Force a ConfigureNotify message so fltk can get the new
- // coordinates after a move of the embedder window.
- if (w)
- w->resize(0,0, w->w(), w->h());
- break;
- case XEMBED_WINDOW_DEACTIVATE:
- break;
- default:
- break;
- }
----

Here the simplified FLTK test program:

---
#include <FL/Fl.H>
#include <FL/Fl_Window.H>
#include <FL/x.H>
#include <cstdio>
#include <iostream>
#include <sstream>
#include <string>

using namespace std;

int main( int argc_, char *argv_[] )
{
Fl_Window dw( 800, 600, "dillo embedded" );
dw.resizable( &dw );
dw.show();
cout << "dw.xid = " << (unsigned long)fl_xid(&dw) << endl;
ostringstream cmd;
cmd << "dillo -f --xid " << (unsigned long)fl_xid(&dw);
if ( argc_ > 1 )
cmd << " " << argv_[1];
cmd << " &";
cout << "system '" << cmd.str() << "'" << endl;
system( cmd.str().c_str() );
return Fl::run();
}
---

[podarcis]

Damn,
something went wrong with pasting in the patch.
Here the complete patch, sorry.

---
diff -r 52633c6c3fc6 src/xembed.cc
--- a/src/xembed.cc Wed Dec 12 19:40:33 2012 +0000
+++ b/src/xembed.cc Sat Dec 15 11:49:23 2012 +0100
@@ -21,93 +21,49 @@

#ifdef X_PROTOCOL

-typedef enum {
- XEMBED_EMBEDDED_NOTIFY = 0,
- XEMBED_WINDOW_ACTIVATE = 1,
- XEMBED_WINDOW_DEACTIVATE = 2,
- XEMBED_REQUEST_FOCUS = 3,
- XEMBED_FOCUS_IN = 4,
- XEMBED_FOCUS_OUT = 5,
- XEMBED_FOCUS_NEXT = 6,
- XEMBED_FOCUS_PREV = 7,
- XEMBED_GRAB_KEY = 8,
- XEMBED_UNGRAB_KEY = 9,
- XEMBED_MODALITY_ON = 10,
- XEMBED_MODALITY_OFF = 11,
-} XEmbedMessageType;
-
-void
-Xembed::setXembedInfo(unsigned long flags)
-{
- unsigned long buffer[2];
-
- Atom xembed_info_atom = XInternAtom (fl_display, "_XEMBED_INFO", false);
-
- buffer[0] = 1;
- buffer[1] = flags;
-
- XChangeProperty (fl_display,
- xid,
- xembed_info_atom, xembed_info_atom, 32,
- PropModeReplace,
- (unsigned char *)buffer, 2);
+void Xembed::update_size() {
+ XWindowAttributes winAttributes;
+ XGetWindowAttributes( fl_display, xid, &winAttributes );
+ int width = winAttributes.width;
+ int height = winAttributes.height;
+ update_size(width, height);
}

-void
-Xembed::sendXembedEvent(uint32_t message) {
- XClientMessageEvent xclient;
-
- memset (&xclient, 0, sizeof (xclient));
- xclient.window = xid;
- xclient.type = ClientMessage;
- xclient.message_type = XInternAtom (fl_display, "_XEMBED", false);
- xclient.format = 32;
- xclient.data.l[0] = fl_event_time;
- xclient.data.l[1] = message;
-
- XSendEvent(fl_display, xid, False, NoEventMask, (XEvent *)&xclient);
- XSync(fl_display, False);
+void Xembed::update_size(int width, int height) {
+ if (w() != width || h() != height)
+ {
+ resize(x(), y(), width, height);
+ redraw();
+ }
}

-int
-Xembed::handle(int e) {
- if (e == FL_PUSH)
- sendXembedEvent(XEMBED_REQUEST_FOCUS);
-
- return Fl_Window::handle(e);
-}
-
+static Xembed *xembed = 0;
static int event_handler(int e, Fl_Window *w) {
- Atom xembed_atom = XInternAtom (fl_display, "_XEMBED", false);
-
- if (fl_xevent->type == ClientMessage) {
- if (fl_xevent->xclient.message_type == xembed_atom) {
- long message = fl_xevent->xclient.data.l[1];
-
- switch (message) {
- case XEMBED_WINDOW_ACTIVATE:
- // Force a ConfigureNotify message so fltk can get the new
- // coordinates after a move of the embedder window.
- if (w)
- w->resize(0,0, w->w(), w->h());
- break;
- case XEMBED_WINDOW_DEACTIVATE:
- break;
- default:
- break;
- }
+ if ( fl_xevent->type == ConfigureNotify )
+ {
+ XConfigureEvent *xcfg = (XConfigureEvent *)fl_xevent;
+ uint32_t xid = xcfg->window;
+ if (xembed && xembed->myxid() == xid)
+ {
+ xembed->update_size(xcfg->width, xcfg->height);
}
}
-
return Fl::handle_(e, w);
}

// TODO: Implement more XEMBED support;

+void Xembed::create() {
+ xembed = this;
+ createInternal(xid);
+ long mask( StructureNotifyMask );
+ XSelectInput(fl_display, xid, mask);
+ Fl::event_dispatch(event_handler);
+}
+
void Xembed::show() {
- createInternal(xid);
- setXembedInfo(1);
- Fl::event_dispatch(event_handler);
+ Fl_Window::show();
+ update_size();
}

void Xembed::createInternal(uint32_t parent) {
diff -r 52633c6c3fc6 src/xembed.hh
--- a/src/xembed.hh Wed Dec 12 19:40:33 2012 +0000
+++ b/src/xembed.hh Sat Dec 15 11:49:23 2012 +0100
@@ -9,15 +9,16 @@
private:
uint32_t xid;
void createInternal(uint32_t parent);
- void setXembedInfo(unsigned long flags);
- void sendXembedEvent(uint32_t message);

public:
Xembed(uint32_t xid, int _w, int _h) : Fl_Window(_w, _h) {
- this->xid = xid;
+ this->xid = xid; create();
};
+ uint32_t myxid() const { return xid; }
+ void create();
void show();
- int handle(int event);
+ void update_size();
+ void update_size( int, int );
};

#endif
---

[Johannes Hofmann]

Hi,

Post by podarcis
Hi,
here is an updated version making resize work.
It still works with tabbed too.
Note that I have had no knowledge of X or xembed at all, so all
changes come from experimenting...
I removed large parts of the existing code, because they did nothing
in my environment.
Please have a look at it.

hm, the patch basically removes support for the xembed protocol. Is
it meant as a diagnostic patch to investigate a problem, or meant
for inclusion into the dillo repository?
It won't work for the latter case. You can't just remove code that
is not needed for your specific case.
If you just want to get embedding of dillo into an fltk based window
working, you would need to send the proper xembed X11 event from your
embedding fltk program. Look at what tabbed is doing.
I can also dig into my old stuff, I think I once had a minimal fltk
based embedder working.

Cheers,
Johannes

[podarcis]

Hi,
well the aim would be to contribute to dillo to make it embeddable by
X11 applications.

If you want to achieve this with "standard" XEMBED protocol only, I am
off -that's not my domain...

I have tested/debugged the current code with 'tabbed' (as you suggested)
and found out, that it works even better if the ClientMessage code (and
this is the only relevant code here) is removed.

tabbed also showed some glitches with the current code:

- dillo's File menu opens at different positions after moving
- clicking in the grey space right to dillo's 'Tools' icon seems to
crash something...

So I removed the whole thing.

Did you try my patch?

It certainly works not only from FLTK applications.
You can verify that, if you try to embed dillo into any window on your
desktop e.g. a gnome terminal (use xwininfo to find out its xid).

I do not claim that my patch is complete already, but it may at least be
used as a starting point. At least it has the advantage, that the
embedder does not need to do anything, except passing an xid.

If keeping the old code is your only concern, then I can re-integrate it
into my patch again - it has no effect on it at all.

Cheers
chris

Post by Johannes Hofmann
Hi,

Post by podarcis
Hi,
here is an updated version making resize work.
It still works with tabbed too.
Note that I have had no knowledge of X or xembed at all, so all
changes come from experimenting...
I removed large parts of the existing code, because they did nothing
in my environment.
Please have a look at it.

hm, the patch basically removes support for the xembed protocol. Is
it meant as a diagnostic patch to investigate a problem, or meant
for inclusion into the dillo repository?
It won't work for the latter case. You can't just remove code that
is not needed for your specific case.
If you just want to get embedding of dillo into an fltk based window
working, you would need to send the proper xembed X11 event from your
embedding fltk program. Look at what tabbed is doing.
I can also dig into my old stuff, I think I once had a minimal fltk
based embedder working.
Cheers,
Johannes
_______________________________________________
Dillo-dev mailing list
http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

[podarcis]

Hi,

I have updated my patch (no removal of existing code, added comments,
trimmed the source,..).

Let me perhaps explain from the beginning:

My starting point was, that when embedding dillo from an external
program using the --xid parameter, dillo's window didn't show up in the
embedder.

I have now found, there's a one-line fix for that problem:

-> dillo_xembed_patch_1.txt

After that was working again (by the way: it worked in dillo2 already) I
tried to make resizing of the embedder window work too:

-> dillo_xembed_patch_2.txt

A test program for all this:

-> dembed.cxx

Looking forward for your comments.

Cheers,
Podarcis


P.S.:
I am aware now, that *real* embedding (to make tab-switching and
focus-handling work together with the embedder) will require the usage
of the xembed-protocol from the embedder side.

I didn't touch the existing part of this implementation.

But I guess embedding dillo without caring for this issues may be useful
too, for example to just display help pages (no focus handling or
tabbing required).

[Johannes Hofmann]

Hi,

Post by podarcis
Hi,
I have updated my patch (no removal of existing code, added
comments, trimmed the source,..).
My starting point was, that when embedding dillo from an external
program using the --xid parameter, dillo's window didn't show up in
the embedder.

Thanks for the updated patches and the background info. Here is some
background info on the xembed stuff from my side:

dillo2 supported xembed more or less automatically as it was based
on gtk2. The claws-mail dillo plugin used this feature to display html.

When porting dillo to fltk we had to implement xembed on our own or
break support for the claws-mail plugin.
That's why we added xembed.cc.

Post by podarcis
-> dillo_xembed_patch_1.txt

I tend to commit this, but I still need to double check that it
doesn't cause any problems.
The advantage is that it will give some initial encouraging result
if someone tries out --xid without a fullblown xembed implementation.

Post by podarcis
After that was working again (by the way: it worked in dillo2
-> dillo_xembed_patch_2.txt
-> dembed.cxx

I would prefer a minimal fltk-based example program in the test
directory, that implements the embedder side of xembed.
It shouldn't be too hard to do (e.g. by looking at the tabbed
sources).
It would be cool if you would like to work on that.
BTW. we use a pretty restricted set of C++ features in the dillo
source tree (e.g. we don't use iostream).

Cheers,
Johannes

[podarcis]

Post by Johannes Hofmann
When porting dillo to fltk we had to implement xembed on our own or
break support for the claws-mail plugin.
That's why we added xembed.cc.

Thank you for the infos. Keeping compatability is of course a main goal...

Nevertheless I am still not really certain, what the current xembed
protocol implementation does - or does *NOT*.

Please have a look at the enclosed diff 'xembed_removed_demo.diff', that
removes the *WHOLE* xembed protocol implementation and only keeps the
window creation.

Still embedding with 'tabbed' works as before. This really makes me
wonder...

Post by Johannes Hofmann
I would prefer a minimal fltk-based example program in the test
directory, that implements the embedder side of xembed.
It shouldn't be too hard to do (e.g. by looking at the tabbed
sources).
It would be cool if you would like to work on that.

Sorry, but I don't feel able to do this proper. Looking at the xembed
protocol specification makes me consider that as an extremely
time-consuming task.

One idea, which could make us both happy: I have again updated my patch,
so that using the xembed protocol can be specified by the application.
If using it (and that's the default of course) will run the current
xembed code effectively unchanged. This way there should be no
compatabiliy issues (The way I have designed that option is only an
outline, maybe this can be done better).
Only if running with xembed protocol disabled my patch comes into action.

--> xembed_patch_xembed_protocol_set.diff
--> dembed.cxx

Cheers,
Podarcis

[podarcis]

Post by podarcis
Please have a look at the enclosed diff 'xembed_removed_demo.diff',
that removes the *WHOLE* xembed protocol implementation and only
keeps the window creation.
Still embedding with 'tabbed' works as before. This really makes me
wonder...

Update:
I just tested Claws Mail (writing this mail with it) from the Ubuntu
12.04 distribution together with the 'Dillo_Viewer' plugin (which was
not so easy to get, because it is not longer in this distribution - I
had to take it from another distibution, openSuse-12.2)

Result:
dillo with the *removed xembed support* (see previous posting)
integrates perfectly (including resizing) into the Claws Mail preview
window!

Also does dillo with *my patch*.

Just to let you know..

Cheers,
podarcis

[Johannes Hofmann]

Post by podarcis

Post by podarcis
Please have a look at the enclosed diff 'xembed_removed_demo.diff',
that removes the *WHOLE* xembed protocol implementation and only
keeps the window creation.
Still embedding with 'tabbed' works as before. This really makes me
wonder...

I just tested Claws Mail (writing this mail with it) from the Ubuntu
12.04 distribution together with the 'Dillo_Viewer' plugin (which was
not so easy to get, because it is not longer in this distribution - I
had to take it from another distibution, openSuse-12.2)
dillo with the *removed xembed support* (see previous posting)
integrates perfectly (including resizing) into the Claws Mail preview
window!
Also does dillo with *my patch*.
Just to let you know..

I know that it works partly with just simple reparenting, but it has
problems. We went through all this see the thread starting with:
http://lists.auriga.wearlab.de/pipermail/dillo-dev/2009-May/006453.html
and the final success messages from claws-mail people here:
http://lists.auriga.wearlab.de/pipermail/dillo-dev/2009-June/006533.html
I don't know if the claws plugin is still used by anyone, but I
don't want to remove the code, that proved to be working.

Cheers,
Johannes

[higuita]

Hi

Post by Johannes Hofmann
I don't know if the claws plugin is still used by anyone, but I
don't want to remove the code, that proved to be working.

I use both dillo and claws-mail, where i load the dillo plugin...
so, yes, it is still used, at least by me :)

Thanks for dillo :)
higuita
--
Naturally the common people don't want war... but after all it is the
leaders of a country who determine the policy, and it is always a
simple matter to drag the people along, whether it is a democracy, or
a fascist dictatorship, or a parliament, or a communist dictatorship.
Voice or no voice, the people can always be brought to the bidding of
the leaders. That is easy. All you have to do is tell them they are
being attacked, and denounce the pacifists for lack of patriotism and
exposing the country to danger. It works the same in every country.
-- Hermann Goering, Nazi and war criminal, 1883-1946