Irc ignore #535

Dunkhan · 2018-09-28T10:18:17Z

This change gives the user an optional field in the advanced settings called "Irc ignore list".
If any nicks are written in here all incoming messages from these will be blocked.

It should be noted that the goal here was not to completely block these nicks but merely to prevent jitsi from conflating chanserv spam with real new messages from actual people. The solution implemented here is overzealous. A perfect solution would still record logs of the messages and also allow the user to initiate pm sessions with these nicks.

Implementing the ideal solution however requires changing far more files, many of which are outside the scope of the irc plugin. As a new committer and an inexperienced programmer I do not feel that it is a good idea for me to implement this as it is a restructuring of the whole messaging workflow.

As it stands this change saves a lot of annoyance for irc users, and is optional and easy to disable for situations where its effects are undesired.

Added support for irc settings in the advanced properties menu

…e old settings were used

ibauersachs

Are you intentionally using a global ignore list and not one specific to an account?

src/net/java/sip/communicator/impl/protocol/irc/MessageManager.java

src/net/java/sip/communicator/impl/protocol/irc/irc.provider.manifest.mf

ibauersachs · 2018-09-28T11:48:02Z

src/net/java/sip/communicator/impl/protocol/irc/irc.provider.manifest.mf

 net.java.sip.communicator.service.certificate,
 net.java.sip.communicator.service.protocol,
 net.java.sip.communicator.service.protocol.event,
 org.slf4j,
 javax.net.ssl,
+ javax.swing,


Same as above.

ibauersachs · 2018-09-28T11:48:44Z

src/net/java/sip/communicator/impl/protocol/irc/properties/IrcIgnoreConfigForm.java

+ *
+ * @author Duncan Robertson
+ */
+public class IrcIgnoreConfigForm


This class must reside in the IrcAccRegWizard package.

ibauersachs · 2018-09-28T11:57:46Z

src/net/java/sip/communicator/impl/protocol/irc/properties/IrcIgnoreConfigForm.java

+    /**
+     * The "ignore" field
+     */
+    private final JTextField ignoreField = new JTextField();


Consider using a list and an input field with button to add/remove entries. Also the Jitsi specific UI components, e.g. a SIPCommTextField.

ibauersachs · 2018-09-28T11:59:49Z

src/net/java/sip/communicator/impl/protocol/irc/properties/IrcProperties.java

+ */
+public class IrcProperties
+{
+    public static final String PROP_IRC_IGNORE = "net.java.sip.communicator.impl.protocol.irc.properties.IGNORE_STRING";


We've never used such a properties class before. While it makes a lot of sense in general, it would need to be in a separate package that contains the IRC 'api', and both the protocol-impl and the account wizard would need to depend on it. So, as dumb as it sounds, please duplicate this constant in the protocol and in the account assistant.

I am still unclear on this. I am not particularly attached to having a properties class. There is only one thing in it. I am not sure where you want me to put it though. What is meant by the protocol and the account assistant?

ibauersachs · 2018-09-28T12:01:39Z

Hi, thanks for your contribution!
If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(

Dunkhan · 2018-09-28T22:24:22Z

Thanks a lot for all the feedback. Any comments about the nature of the feature itself? I am still not entirely comfortable with it. I definitely want it in my own client but I am not sure how many other people it will be useful to. I feel like there needs to be a silence feature that might need to be independent of the protocols. That makes it so when you get messages it does not show any notifications. The issue is if there are large chatrooms open, the new message highlight is always on, and if an important or urgent personal message comes through on another protocol it often goes unnoticed for hours.

Having said that, such a feature is a lot more work and I don't even know exactly how to do it. A silence on/off status on every permanent chatroom object would be needed, and a silence status on contacts as well, separate from blocked status. The bots this pr deals with are not even stored as permanent contacts so there would have to be a mechanism for making them persistent automatically so they could have status preferences saved on them at all.

Regardless of all this. I will make the suggested changes, I can always revert all this if I one day get into more in-depth development and make a better version of the feature.

ibauersachs · 2018-09-29T07:53:44Z

The feature itself seems useful, I'll be one of the first users to block Freenode's annoying messages when returning from sleep. An independent block user/chatroom/bot feature would definitely be better, but also more complicated to implement. Up to you to give it a go.

As for your other question for the UI removal from the protocol: the ircaccregwiz package is a better option for the config panel. The protocol package should remain headless (i.e. able to run on servers).

Modified config form to have a list with add and remove buttons

Dunkhan · 2018-10-03T11:38:23Z

what do I do with signed CLA once I have it?

ibauersachs · 2018-10-13T14:05:13Z

resources/languages/resources.properties

@@ -1031,6 +1031,9 @@ plugin.ircaccregwizz.SASL_USERNAME=Username
 plugin.ircaccregwizz.SASL_AUTHZ_ROLE=Role
 plugin.ircaccregwizz.SASL_IRC_PASSWORD_USED=IRC password above is used for SASL authentication
 plugin.ircaccregwizz.RESOLVE_DNS_THROUGH_PROXY=Always resolve DNS names through proxy
+plugin.irc.IRC_IGNORE_DESCRIPTION=<html> Add the names of contacts whose messages should not open a \
+new window or trigger notifications. e.g. chanserv</html>
+plugin.irc.IRC_IGNORE_CONFIG=Irc ignore list


IRC is written uppercase erverywhere else

ibauersachs · 2018-10-13T14:05:24Z

resources/languages/resources.properties

@@ -1766,4 +1769,4 @@ plugin.propertieseditor.NEED_RESTART=Note: some properties might need a restart
 #Thunderbird address book plugin
 plugin.thunderbird.CONFIG_FORM_TITLE=Thunderbird
 plugin.thunderbird.ENABLED=Enabled
-plugin.thunderbird.PHONE_PREFIX=Phone prefix:
+plugin.thunderbird.PHONE_PREFIX=Phone prefix:


Files should end with a newline.

ibauersachs · 2018-10-13T14:06:51Z

src/net/java/sip/communicator/impl/protocol/irc/irc.provider.manifest.mf

@@ -10,11 +10,11 @@ Import-Package: org.osgi.framework,
 org.jitsi.service.resources,
 net.java.sip.communicator.service.resources,
 net.java.sip.communicator.util,
+ net.java.sip.communicator.plugin.desktoputil,


The import shouldn't be here anymore.

ibauersachs · 2018-10-13T14:07:40Z

src/net/java/sip/communicator/impl/protocol/irc/irc.provider.manifest.mf

 net.java.sip.communicator.service.certificate,
 net.java.sip.communicator.service.protocol,
 net.java.sip.communicator.service.protocol.event,
 org.slf4j,
- javax.net.ssl,


Are you sure this isn't needed?

ibauersachs · 2018-10-13T14:08:15Z

src/net/java/sip/communicator/impl/protocol/irc/properties/IrcProperties.java

+{
+    public static final String PROP_IRC_IGNORE = "net.java.sip.communicator.impl.protocol.irc.properties.IGNORE_STRING";
+
+    public IrcProperties()


This constructor is redundant. If at all, make it private to mark IrcProperties as a utility class.

ibauersachs · 2018-10-13T14:10:04Z

src/net/java/sip/communicator/plugin/ircaccregwizz/IrcIgnoreConfigForm.java

@@ -0,0 +1,215 @@
+package net.java.sip.communicator.plugin.ircaccregwizz;
+
+import net.java.sip.communicator.impl.protocol.irc.properties.*;


You'll need to copy the property. The ircaccregwiz is a different bundle and cannot use anthing from the protocol impl.

ibauersachs · 2018-10-13T14:11:48Z

Damencho will check that they've received the CLA. You don't have to do anything else.

damencho · 2018-10-15T00:07:58Z

I don't see the CLA in the list of signed ones, the last update was on Oct 09, 2018.
@Dunkhan have you submitted it to DocuSign?

Dunkhan added 2 commits September 16, 2018 01:50

Added the ability to ignore spamming server bots on irc

a2cf128

Added support for irc settings in the advanced properties menu

Corrected the format settings in my ide and fixed the places where th…

a6fb83d

…e old settings were used

ibauersachs requested changes Sep 28, 2018

View reviewed changes

ibauersachs added component:Chat cla-missing protocol:IRC labels Sep 28, 2018

Moved config form to account registration wizard package

c2deffa

Modified config form to have a list with add and remove buttons

ibauersachs requested changes Oct 13, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Irc ignore #535

Irc ignore #535

Dunkhan commented Sep 28, 2018

ibauersachs left a comment

ibauersachs Sep 28, 2018

ibauersachs Sep 28, 2018

ibauersachs Sep 28, 2018

ibauersachs Sep 28, 2018

Dunkhan Oct 3, 2018

ibauersachs commented Sep 28, 2018

Dunkhan commented Sep 28, 2018 •

edited

ibauersachs commented Sep 29, 2018

Dunkhan commented Oct 3, 2018

ibauersachs Oct 13, 2018

ibauersachs Oct 13, 2018

ibauersachs Oct 13, 2018

ibauersachs Oct 13, 2018

ibauersachs Oct 13, 2018

ibauersachs Oct 13, 2018

ibauersachs commented Oct 13, 2018

damencho commented Oct 15, 2018

		@@ -0,0 +1,215 @@
		package net.java.sip.communicator.plugin.ircaccregwizz;

		import net.java.sip.communicator.impl.protocol.irc.properties.*;

Irc ignore #535

Are you sure you want to change the base?

Irc ignore #535

Conversation

Dunkhan commented Sep 28, 2018

ibauersachs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibauersachs commented Sep 28, 2018

Dunkhan commented Sep 28, 2018 • edited

ibauersachs commented Sep 29, 2018

Dunkhan commented Oct 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibauersachs commented Oct 13, 2018

damencho commented Oct 15, 2018

Dunkhan commented Sep 28, 2018 •

edited