[sip-comm-dev] Mystery plugin error


#1

Hi all,

There seems to be a mysterious plugin bundle in SC. It shows up in the Plugin config page as "unknown." I've tried to find it by process of elimination, by eliminating all the bundles that are listed elsewhere in the list, but I seem to have eliminated everything. I also believe that this is the cause of the following error when SC loads:

12:14:36.266 SEVERE: impl.gui.UIServiceImpl.serviceChanged().667 Plugin Component type is not supported.Should provide a plugin in AWT, SWT or Swing.

Anyone have any idea what's going on here?

Thanks,

Alan

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#2

Hi Alan,

There seems to be a mysterious plugin bundle in SC. It shows up in the Plugin config page as "unknown." I've tried to find it by process of elimination, by eliminating all the bundles that are listed elsewhere in the list, but I seem to have eliminated everything.

This is caused by the log4j bundle not having a bundle name. When
trying to solve this issue, I found new bugs:
1) lib/bundle is not deleted by ant clean
2) lib/bundle JARs are not regenerated by ant make
3) lib/servicebinder.jar and
lib/installer-exclude/architectureviewer1.1.jar are missing
4) lib/servicebinder.jar should be moved to
lib/installer-exclude/servicebinder.jar
5) lib/bundle shouldn't be committed to the svn repository
6) there should be a ${lib.dest} == "${lib}/bundle" variable in build.xml

The patch attached solves all these issues (+ log4j bundle name of
course) but 3) 4) 5).

12:14:36.266 SEVERE: impl.gui.UIServiceImpl.serviceChanged().667 Plugin Component type is not supported.Should provide a plugin in AWT, SWT or Swing.

Anyone have any idea what's going on here?

The culprit is net.java.sip.communicator.plugin.whiteboard.WhiteboardMenuItem.
Its getComponent() method returns null if there is no reason to
display the whiteboard menu item in the contact context menu and this
confuses the UI service when registering the plugin component.

We could make WhiteMenuItem.getComponent() always return the menu item
and that would solve the problem. But that would always display the
whiteboard menu item even when it's irrelevant. A better solution
would be to add a getComponentClass() method to the PluginComponent
interface which would always return the class of the
would-be-displayed component.

Cheers,

build.xml_fixes.patch (3.35 KB)

···

On Tue, Aug 19, 2008 at 6:23 PM, Alan C Kelly <akelly7@gmu.edu> wrote:

--
Sébastien Mazy


#3

Hi Sébastien,

This is caused by the log4j bundle not having a bundle name. When
trying to solve this issue, I found new bugs:
1) lib/bundle is not deleted by ant clean
2) lib/bundle JARs are not regenerated by ant make
3) lib/servicebinder.jar and
lib/installer-exclude/architectureviewer1.1.jar are missing
4) lib/servicebinder.jar should be moved to
lib/installer-exclude/servicebinder.jar
5) lib/bundle shouldn't be committed to the svn repository
6) there should be a ${lib.dest} == "${lib}/bundle" variable in
build.xml
The patch attached solves all these issues (+ log4j bundle name of
course) but 3) 4) 5).

Good work! What a mess... cleaning the rest of that up sounds like a problem for the committers.

The culprit is
net.java.sip.communicator.plugin.whiteboard.WhiteboardMenuItem.Its
getComponent() method returns null if there is no reason to
display the whiteboard menu item in the contact context menu and this
confuses the UI service when registering the plugin component.

I thought it might be the Whiteboard but I fixed what I thought was the problem and it still didn't work. I'll see about patching this now.

Thanks for the help!

Alan


#4

Hi Sébastien,

A better solution
would be to add a getComponentClass() method to the PluginComponent
interface which would always return the class of the
would-be-displayed component.

I'm afraid I missed your point. I don't understand exactly how this would work. How would adding a getComponentClass() method solve the problem? What would it be used for?

Also, after another look at the code, it looks like the plugin should work the way it is intended, it just causes an error to be placed in the log because the logger assumes that a null value is an error. The menu item should be properly added to a contact when there is a Whiteboard available. Maybe we just need to change that logger output (or ignore it).

Thanks for your help,

Alan


#5

Hey Seb,

Apologies for not replying earlier.

(more inline)

Sébastien Mazy написа:

Hi Alan,

There seems to be a mysterious plugin bundle in SC. It shows up in the Plugin config page as "unknown." I've tried to find it by process of elimination, by eliminating all the bundles that are listed elsewhere in the list, but I seem to have eliminated everything.

This is caused by the log4j bundle not having a bundle name. When
trying to solve this issue, I found new bugs:
1) lib/bundle is not deleted by ant clean
2) lib/bundle JARs are not regenerated by ant make
3) lib/servicebinder.jar and
lib/installer-exclude/architectureviewer1.1.jar are missing
4) lib/servicebinder.jar should be moved to
lib/installer-exclude/servicebinder.jar
5) lib/bundle shouldn't be committed to the svn repository
6) there should be a \{lib\.dest\} == &quot;{lib}/bundle" variable in build.xml

Well actually the lib directory is supposed to be immutable and
lib/bundle is hence a container for third-party bundles. As such they
are treated as the rest of our libs and they get to live in our SVN
repository, and are also not supposed to be regenerated nor cleaned
since they don't get modified during development.

We do have utility targets (I guess this is what led to the confusion)
that allow us to regenerate most of them should we replace them with
newer versions but this is supposed to happen only very rarely.

Makes sense?

Emil

···

On Tue, Aug 19, 2008 at 6:23 PM, Alan C Kelly <akelly7@gmu.edu> wrote:

The patch attached solves all these issues (+ log4j bundle name of
course) but 3) 4) 5).

12:14:36.266 SEVERE: impl.gui.UIServiceImpl.serviceChanged().667 Plugin Component type is not supported.Should provide a plugin in AWT, SWT or Swing.

Anyone have any idea what's going on here?

The culprit is net.java.sip.communicator.plugin.whiteboard.WhiteboardMenuItem.
Its getComponent() method returns null if there is no reason to
display the whiteboard menu item in the contact context menu and this
confuses the UI service when registering the plugin component.

We could make WhiteMenuItem.getComponent() always return the menu item
and that would solve the problem. But that would always display the
whiteboard menu item even when it's irrelevant. A better solution
would be to add a getComponentClass() method to the PluginComponent
interface which would always return the class of the
would-be-displayed component.

Cheers,

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#6

Hi Alan,

I'm afraid I missed your point. I don't understand exactly how this would work. How would adding a getComponentClass() method solve the problem? What would it be used for?

Also, after another look at the code, it looks like the plugin should work the way it is intended, it just causes an error to be placed in the log because the logger assumes that a null value is an error. The menu item should be properly added to a contact when there is a Whiteboard available. Maybe we just need to change that logger output (or ignore it).

This isn't just a log issue. Currently the whiteboard menu item is
never added to the contact menu because its regitration fails (caused
by WhiteboardMenuItem.getComponent() returning null at registration
time). However, I don't think WhiteboardMenuItem.getComponent() should
be changed. It's logical to return null when you don't wan't the
component to be displayed.

Adding a PluginComponent.getComponentClass() would have returned the
class of the object returned by getComponent() *when it's not null*,
so that the registration wouldn't have failed. But now, I think it's a
bad idea. It will be simpler to drop the getComponent() checks from
UIServiceImpl.serviceChanged() and let the PluginListenerS perform
their own checks. After all, they are the most concerned by what
getComponent() returns.

Cheers,

···

On Wed, Aug 20, 2008 at 8:52 PM, Alan C Kelly <akelly7@gmu.edu> wrote:

--
Sébastien Mazy


#7

Hi,

Sébastien Mazy написа:

This is caused by the log4j bundle not having a bundle name. When
trying to solve this issue, I found new bugs:
1) lib/bundle is not deleted by ant clean
2) lib/bundle JARs are not regenerated by ant make
3) lib/servicebinder.jar and
lib/installer-exclude/architectureviewer1.1.jar are missing
4) lib/servicebinder.jar should be moved to
lib/installer-exclude/servicebinder.jar
5) lib/bundle shouldn't be committed to the svn repository
6) there should be a ${lib.dest} == "${lib}/bundle" variable in build.xml

Well actually the lib directory is supposed to be immutable and
lib/bundle is hence a container for third-party bundles. As such they
are treated as the rest of our libs and they get to live in our SVN
repository, and are also not supposed to be regenerated nor cleaned
since they don't get modified during development.

We do have utility targets (I guess this is what led to the confusion)
that allow us to regenerate most of them should we replace them with
newer versions but this is supposed to happen only very rarely.

Makes sense?

From what I understand, some JARs from lib/installer-exclude/ have to
be "deployed" in lib/bundles because then need a different manifest to
suit OSGi. IMHO if you change one of these manifests like I did line
82 of build.xml_fixes.patch (see previous mails), you could expect
that an "ant rebuild" would acknowledge that. I haven't timed it, but
deploying a few JARs from one directory to another shouldn't be too
costly. Moreover, it would avoid to have the same classes twice in the
svn repo.

However, as it's a different issue, I've attached a patch which only
solves the "no bundle name" problem. Don't forget to run "ant
bundle-log4j" after you apply it ;). Tested on svn revision 4398.

no_bundle_name.patch (712 Bytes)

···

On Sun, Sep 7, 2008 at 11:52 PM, Emil Ivov <emcho@sip-communicator.org> wrote:

--
Sébastien Mazy


#8

Hi Sébastien,

What you're saying makes sense. However, now that I've played with it, I'm not sure that it needs to return null when the whiteboard is unavailable. I commented that part out and replaced with a simple return true. The plugin works very nicely: It greys out the whiteboard menu option when the whiteboard is unavailable and changes the tooltip to say that the contact does not support whiteboarding.

I'm inclined to think that this is preferable, rather than a case where a user installs the plugin and then is frustrated that the menu option never shows up. This way it's clear that your contact doesn't support the feature. There are several other plugins that work this way.

That's just my personal preference, you may disagree :wink: Perhaps we should ask for feedback on this.

It turns out that there were a number of other problems with that plugin. I have fixed it and am attaching a patch file, in case you'd like to look at it.

Alan

whiteboard-menu-item-2.patch (2.94 KB)


#9

The plugin works very nicely: It greys out the whiteboard menu option when the whiteboard is unavailable and changes the tooltip to say that the contact does not support whiteboarding.

I'm inclined to think that this is preferable, rather than a case where a user installs the plugin and then is frustrated that the menu option never shows up. This way it's clear that your contact doesn't support the feature. There are several other plugins that work this way.

I'm not a usuability expert but your solution seems better than
deleting the menu entry (and as you said, it is consistent with what's
done in the other plugins). Moreover, it solves the problem in a KISS
way (Keep It Simple Stupid).

It turns out that there were a number of other problems with that plugin. I have fixed it and am attaching a patch file, in case you'd like to look at it.

A few remarks:
- you removed the duplicate iteration on the metacontact's contacts -> OK
- you replaced the JMenu by a JMenuItem -> NOK

See the corrective patch attached (applies to svn revision 4313).

Cheers,

whiteboard-menu-item-3.patch (1.22 KB)

···

On Thu, Aug 21, 2008 at 12:02 AM, Alan C Kelly <akelly7@gmu.edu> wrote:

--
Sébastien Mazy


#10

Hey Seb,

Sébastien Mazy написа:

From what I understand, some JARs from lib/installer-exclude/ have to
be "deployed" in lib/bundles because then need a different manifest to
suit OSGi.

OK I see what's causing the confusion and you do have point. The point
of installer-exclude is to serve as a container directory for jars that
should be ignored by our installation packages (deb, izpack, dmg, etc)
because they are deployed in some other form. Most often this is because
the lib is being included inside one of our own sc-bundles. This is
exactly the case with log4j and that's why in there.

As I already mentioned lib/bundles contains bundles that we don't
generate. They are supposed to be provided by a third party and the
directory is therefore meant to be "static". In other words, we don't
clean it and our build process does not put anything dynamically
generated in there.

The confusion comes from the fact that we consider the log4j bundle as a
thirdparty lib, which is why it also lives in lib/bundles.

I haven't timed it, but
deploying a few JARs from one directory to another shouldn't be too
costly. Moreover, it would avoid to have the same classes twice in the
svn repo.

Agreed. Would you like to change build.xml so that generation of the
log4j bundle would be executed on every make? Besides, we seem to be
including the lib in some of our other bundles which is redundant and
they should also be using the standalone bundle. Want to change this too?

However, as it's a different issue, I've attached a patch which only
solves the "no bundle name" problem. Don't forget to run "ant
bundle-log4j" after you apply it ;). Tested on svn revision 4398.

Commited and acked! Thanks Seb!

Cheers
Emil

···

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#11

you replaced the JMenu by a JMenuItem -> NOK

Oh, that's interesting :x

The Menu didn't work properly when I tested it before so I totally missed the significance of the JMenu. It was placing the contacts directly on the top level menu. And I thought my IDE was giving an error that said that a PluginComponent had to extend a GUI element. I don't know what's different now... weird.

Wow I'm confused and embarrassed.... Sorry!

I think I have an idea how to make this more intuitive, I'll keep looking at it.

Alan


#12

Sébastien,

The attached patch should idiot-proof this plugin (idiots like me ;p) and also avoids the return null problem.

If there are no contacts that support the Whiteboard it will return a disabled "Whiteboard" JMenuItem, instead of returning null or returning the JMenu with a list of disabled JMenuItems. This way we don't add a useless menu, but the user is still informed that whiteboarding is unsupported.

Thanks for your help,

Alan

whiteboard-menu-item-4.patch (1.58 KB)


#13

Hi Alan,

···

On Thu, Aug 21, 2008 at 5:48 PM, Alan C Kelly <akelly7@gmu.edu> wrote:

The attached patch should idiot-proof this plugin (idiots like me ;p) and also avoids the return null problem.

If there are no contacts that support the Whiteboard it will return a disabled "Whiteboard" JMenuItem, instead of returning null or returning the JMenu with a list of disabled JMenuItems. This way we don't add a useless menu, but the user is still informed that whiteboarding is unsupported.

I tested it and your patch seems fine to me (detail: remove the
trailing whitespaces).

Cheers,

--
Sébastien Mazy


#14

Hey folks,

I am very sorry for not replying to this earlier. I completely missed
the discussion and only discover it now.

Alan, I have committed a fix that was inspired by your patch (now acked
on team & contributors) with the only difference being that we now use a
single menu object to handle both cases.

Many of us have been receiving a decent amount of offline scolding by
Lubomir Marinov (greatly appreciated!) on the subject of wasting memory,
so given that swing objects tend to be relatively greedy I thought we'd
better only keep one of them.

Can you please have a look and let me know what you think?

Cheers
Emil

Sébastien Mazy написа:

···

Hi Alan,

On Thu, Aug 21, 2008 at 5:48 PM, Alan C Kelly <akelly7@gmu.edu> wrote:

The attached patch should idiot-proof this plugin (idiots like me ;p) and also avoids the return null problem.

If there are no contacts that support the Whiteboard it will return a disabled "Whiteboard" JMenuItem, instead of returning null or returning the JMenu with a list of disabled JMenuItems. This way we don't add a useless menu, but the user is still informed that whiteboarding is unsupported.

I tested it and your patch seems fine to me (detail: remove the
trailing whitespaces).

Cheers,

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#15

Ok thanks, I'll have a look when I get a chance.

···

-----Original Message-----

From: Emil Ivov [mailto:emcho@sip-communicator.org]

Sent: Sunday, September 07, 2008 5:53 PM
To: dev@sip-communicator.dev.java.net
Subject: Re: [sip-comm-dev] Mystery plugin error

Hey folks,

I am very sorry for not replying to this earlier. I completely missed the discussion and only discover it now.

Alan, I have committed a fix that was inspired by your patch (now acked on team & contributors) with the only difference being that we now use a single menu object to handle both cases.

Many of us have been receiving a decent amount of offline scolding by Lubomir Marinov (greatly appreciated!) on the subject of wasting memory, so given that swing objects tend to be relatively greedy I thought we'd better only keep one of them.

Can you please have a look and let me know what you think?

Cheers
Emil

Sébastien Mazy написа:

Hi Alan,

On Thu, Aug 21, 2008 at 5:48 PM, Alan C Kelly <akelly7@gmu.edu> wrote:

The attached patch should idiot-proof this plugin (idiots like me ;p) and also avoids the return null problem.

If there are no contacts that support the Whiteboard it will return a disabled "Whiteboard" JMenuItem, instead of returning null or returning the JMenu with a list of disabled JMenuItems. This way we don't add a useless menu, but the user is still informed that whiteboarding is unsupported.

I tested it and your patch seems fine to me (detail: remove the
trailing whitespaces).

Cheers,

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#16

It looks good to me. Thanks!

···

----- Original Message -----

From: Alan Kelly <akelly7@gmu.edu>

Date: Monday, September 8, 2008 11:00 am
Subject: RE: [sip-comm-dev] Mystery plugin error

Ok thanks, I'll have a look when I get a chance.

-----Original Message-----
From: Emil Ivov [mailto:emcho@sip-communicator.org]
Sent: Sunday, September 07, 2008 5:53 PM
To: dev@sip-communicator.dev.java.net
Subject: Re: [sip-comm-dev] Mystery plugin error

Hey folks,

I am very sorry for not replying to this earlier. I completely
missed the discussion and only discover it now.

Alan, I have committed a fix that was inspired by your patch (now
acked on team & contributors) with the only difference being that
we now use a single menu object to handle both cases.

Many of us have been receiving a decent amount of offline scolding
by Lubomir Marinov (greatly appreciated!) on the subject of
wasting memory, so given that swing objects tend to be relatively
greedy I thought we'd better only keep one of them.

Can you please have a look and let me know what you think?

Cheers
Emil

Sébastien Mazy написа:
> Hi Alan,
>
> On Thu, Aug 21, 2008 at 5:48 PM, Alan C Kelly <akelly7@gmu.edu>
wrote:>> The attached patch should idiot-proof this plugin (idiots
like me ;p) and also avoids the return null problem.
>>
>> If there are no contacts that support the Whiteboard it will
return a disabled "Whiteboard" JMenuItem, instead of returning
null or returning the JMenu with a list of disabled JMenuItems.
This way we don't add a useless menu, but the user is still
informed that whiteboarding is unsupported.
>
> I tested it and your patch seems fine to me (detail: remove the
> trailing whitespaces).
>
> Cheers,
>

-------------------------------------------------------------------
--
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-
communicator.dev.java.net

-------------------------------------------------------------------
--
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-
communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net