[jitsi-dev] [jitsi-commits] master: Escape html entities for plain text messages in notifications. (baeca0e)


#1

Hey Danny,

what is the problem that you solved with this one? Asking cause it
seems on macosx escapes some special chars and the escaped string is
seen in the growl notification (like é -> é) and I don't want
to break your stuff and just to check for appropriate fix.

Regards
damencho

···

On Mon, Jan 26, 2015 at 12:13 AM, <danny@dannyvanheumen.nl> wrote:

Repository : ssh://lists.jitsi.org/jitsi

On branch : master
Link : https://github.com/jitsi/jitsi/compare/e547c42c44f8a3efae495c4be27d5e1505f59b21...baeca0ef7bdd708c92ca67724ba5cb9985bdb332

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

commit baeca0ef7bdd708c92ca67724ba5cb9985bdb332
Author: Danny van Heumen <danny@dannyvanheumen.nl>
Date: Sun Jan 25 17:11:38 2015 +0100

    Escape html entities for plain text messages in notifications.

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

baeca0ef7bdd708c92ca67724ba5cb9985bdb332
build.xml | 2 +-
.../notificationwiring/NotificationManager.java | 51 ++++++++++++++++++----
.../notificationwiring.manifest.mf | 4 +-
3 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/build.xml b/build.xml
index af8c0e6..f84917a 100644
--- a/build.xml
+++ b/build.xml
@@ -2178,7 +2178,7 @@ javax.swing.event, javax.swing.border"/>
     </target>

     <!-- BUNDLE-NOTIFICATION-WIRING -->
- <target name="bundle-notification-wiring">
+ <target name="bundle-notification-wiring" depends="bundle-commons-lang">
         <!-- Creates a bundle for the notifications.-->
         <jar compress="false" destfile="${bundles.dest}/notification-wiring.jar"
             manifest="${src}/net/java/sip/communicator/plugin/notificationwiring/notificationwiring.manifest.mf">
diff --git a/src/net/java/sip/communicator/plugin/notificationwiring/NotificationManager.java b/src/net/java/sip/communicator/plugin/notificationwiring/NotificationManager.java
index 1830282..6390529 100644
--- a/src/net/java/sip/communicator/plugin/notificationwiring/NotificationManager.java
+++ b/src/net/java/sip/communicator/plugin/notificationwiring/NotificationManager.java
@@ -18,6 +18,7 @@ import net.java.sip.communicator.service.protocol.event.*;
import net.java.sip.communicator.service.resources.*;
import net.java.sip.communicator.util.*;

+import org.apache.commons.lang3.*;
import org.jitsi.service.neomedia.*;
import org.jitsi.service.neomedia.recording.*;
import org.jitsi.service.protocol.event.*;
@@ -99,6 +100,11 @@ public class NotificationManager
     public static final String INCOMING_MESSAGE = "IncomingMessage";

     /**
+ * HTML content type.
+ */
+ private static final String HTML_CONTENT_TYPE = "text/html";
+
+ /**
      * The <tt>Logger</tt> used by the <tt>NotificationManager</tt> class and
      * its instances for logging output.
      */
@@ -130,6 +136,7 @@ public class NotificationManager
      * @param eventType the event type for which we fire a notification
      * @param messageTitle the title of the message
      * @param message the content of the message
+ * @param messageUID the message UID
      */
     public static void fireChatNotification(Object chatContact,
                                             String eventType,
@@ -1145,12 +1152,20 @@ public class NotificationManager
                     = NotificationWiringActivator.getResources().getI18NString(
                             "service.gui.MSG_RECEIVED",
                             new String[] { sourceParticipant.getDisplayName() });
-
+ final String htmlContent;
+ if (HTML_CONTENT_TYPE.equals(evt.getMessage().getContentType()))
+ {
+ htmlContent = messageContent;
+ }
+ else
+ {
+ htmlContent = StringEscapeUtils.escapeHtml4(messageContent);
+ }
                 fireChatNotification(
                     sourceChatRoom,
                     INCOMING_MESSAGE,
                     title,
- messageContent,
+ htmlContent,
                     evt.getMessage().getMessageUID());
             }
         }
@@ -1178,7 +1193,8 @@ public class NotificationManager
             // Fire notification
             boolean fireChatNotification;

- String messageContent = evt.getMessage().getContent();
+ final Message sourceMsg = evt.getMessage();
+ String messageContent = sourceMsg.getContent();

             /*
              * It is uncommon for IRC clients to display popup notifications for
@@ -1212,13 +1228,21 @@ public class NotificationManager
                     = NotificationWiringActivator.getResources().getI18NString(
                         "service.gui.MSG_RECEIVED",
                         new String[] { sourceMember.getName() });
-
+ final String htmlContent;
+ if (HTML_CONTENT_TYPE.equals(sourceMsg.getContentType()))
+ {
+ htmlContent = messageContent;
+ }
+ else
+ {
+ htmlContent = StringEscapeUtils.escapeHtml4(messageContent);
+ }
                 fireChatNotification(
                         sourceChatRoom,
                         INCOMING_MESSAGE,
                         title,
- messageContent,
- evt.getMessage().getMessageUID());
+ htmlContent,
+ sourceMsg.getMessageUID());
             }
         }
         catch(Throwable t)
@@ -1241,12 +1265,23 @@ public class NotificationManager
                 "service.gui.MSG_RECEIVED",
                 new String[]{evt.getSourceContact().getDisplayName()});

+ final Message sourceMsg = evt.getSourceMessage();
+ final String htmlContent;
+ if (HTML_CONTENT_TYPE.equals(sourceMsg.getContentType()))
+ {
+ htmlContent = sourceMsg.getContent();
+ }
+ else
+ {
+ htmlContent =
+ StringEscapeUtils.escapeHtml4(sourceMsg.getContent());
+ }
             fireChatNotification(
                     evt.getSourceContact(),
                     INCOMING_MESSAGE,
                     title,
- evt.getSourceMessage().getContent(),
- evt.getSourceMessage().getMessageUID());
+ htmlContent,
+ sourceMsg.getMessageUID());
         }
         catch(Throwable t)
         {
diff --git a/src/net/java/sip/communicator/plugin/notificationwiring/notificationwiring.manifest.mf b/src/net/java/sip/communicator/plugin/notificationwiring/notificationwiring.manifest.mf
index 46214fb..5f3a542 100644
--- a/src/net/java/sip/communicator/plugin/notificationwiring/notificationwiring.manifest.mf
+++ b/src/net/java/sip/communicator/plugin/notificationwiring/notificationwiring.manifest.mf
@@ -17,5 +17,5 @@ Import-Package: javax.imageio,
  org.jitsi.service.neomedia.recording,
  org.jitsi.service.protocol.event,
  org.jitsi.service.resources,
- org.osgi.framework
-
+ org.osgi.framework,
+ org.apache.commons.lang3

_______________________________________________
commits mailing list
commits@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/commits


#2

Hi damencho,

Basically, html injection in notification window. Got this issue for
messages that were plain text, arrived as plain text at the listener and
then got displayed as html-interpreted, so "<b>dude</b>" shows as "dude" in
bold.

Can you check if message type is indeed html when you get the &eacute; in
the notification? (Because I don't think I intended to change anything in
case of "normal" HTML messages.)

Danny

···

On Mon, Feb 16, 2015 at 8:56 AM, Damian Minkov <damencho@jitsi.org> wrote:

Hey Danny,

what is the problem that you solved with this one? Asking cause it
seems on macosx escapes some special chars and the escaped string is
seen in the growl notification (like é -> &eacute;) and I don't want
to break your stuff and just to check for appropriate fix.

Regards
damencho

On Mon, Jan 26, 2015 at 12:13 AM, <danny@dannyvanheumen.nl> wrote:
> Repository : ssh://lists.jitsi.org/jitsi
>
> On branch : master
> Link :
https://github.com/jitsi/jitsi/compare/e547c42c44f8a3efae495c4be27d5e1505f59b21...baeca0ef7bdd708c92ca67724ba5cb9985bdb332
>
>>---------------------------------------------------------------
>
> commit baeca0ef7bdd708c92ca67724ba5cb9985bdb332
> Author: Danny van Heumen <danny@dannyvanheumen.nl>
> Date: Sun Jan 25 17:11:38 2015 +0100
>
> Escape html entities for plain text messages in notifications.
>
>
>>---------------------------------------------------------------
>
> baeca0ef7bdd708c92ca67724ba5cb9985bdb332
> build.xml | 2 ±
> …/notificationwiring/NotificationManager.java | 51
+++++++++++++++++±—
> …/notificationwiring.manifest.mf | 4 ±
> 3 files changed, 46 insertions(+), 11 deletions(-)
>
> diff --git a/build.xml b/build.xml
> index af8c0e6…f84917a 100644
> — a/build.xml
> +++ b/build.xml
> @@ -2178,7 +2178,7 @@ javax.swing.event, javax.swing.border"/>
> </target>
>
> <!-- BUNDLE-NOTIFICATION-WIRING -->
> - <target name="bundle-notification-wiring">
> + <target name="bundle-notification-wiring"
depends="bundle-commons-lang">
> <!-- Creates a bundle for the notifications.–>
> <jar compress="false"
destfile="${bundles.dest}/notification-wiring.jar"
>
manifest="${src}/net/java/sip/communicator/plugin/notificationwiring/notificationwiring.manifest.mf">
> diff --git
a/src/net/java/sip/communicator/plugin/notificationwiring/NotificationManager.java
b/src/net/java/sip/communicator/plugin/notificationwiring/NotificationManager.java
> index 1830282…6390529 100644
> —
a/src/net/java/sip/communicator/plugin/notificationwiring/NotificationManager.java
> +++
b/src/net/java/sip/communicator/plugin/notificationwiring/NotificationManager.java
> @@ -18,6 +18,7 @@ import
net.java.sip.communicator.service.protocol.event.*;
> import net.java.sip.communicator.service.resources.*;
> import net.java.sip.communicator.util.*;
>
> +import org.apache.commons.lang3.*;
> import org.jitsi.service.neomedia.*;
> import org.jitsi.service.neomedia.recording.*;
> import org.jitsi.service.protocol.event.*;
> @@ -99,6 +100,11 @@ public class NotificationManager
> public static final String INCOMING_MESSAGE = "IncomingMessage";
>
> /**
> + * HTML content type.
> + */
> + private static final String HTML_CONTENT_TYPE = "text/html";
> +
> + /**
> * The <tt>Logger</tt> used by the <tt>NotificationManager</tt>
class and
> * its instances for logging output.
> */
> @@ -130,6 +136,7 @@ public class NotificationManager
> * @param eventType the event type for which we fire a notification
> * @param messageTitle the title of the message
> * @param message the content of the message
> + * @param messageUID the message UID
> */
> public static void fireChatNotification(Object chatContact,
> String eventType,
> @@ -1145,12 +1152,20 @@ public class NotificationManager
> =
NotificationWiringActivator.getResources().getI18NString(
> "service.gui.MSG_RECEIVED",
> new String[] {
sourceParticipant.getDisplayName() });
> -
> + final String htmlContent;
> + if
(HTML_CONTENT_TYPE.equals(evt.getMessage().getContentType()))
> + {
> + htmlContent = messageContent;
> + }
> + else
> + {
> + htmlContent =
StringEscapeUtils.escapeHtml4(messageContent);
> + }
> fireChatNotification(
> sourceChatRoom,
> INCOMING_MESSAGE,
> title,
> - messageContent,
> + htmlContent,
> evt.getMessage().getMessageUID());
> }
> }
> @@ -1178,7 +1193,8 @@ public class NotificationManager
> // Fire notification
> boolean fireChatNotification;
>
> - String messageContent = evt.getMessage().getContent();
> + final Message sourceMsg = evt.getMessage();
> + String messageContent = sourceMsg.getContent();
>
> /*
> * It is uncommon for IRC clients to display popup
notifications for
> @@ -1212,13 +1228,21 @@ public class NotificationManager
> =
NotificationWiringActivator.getResources().getI18NString(
> "service.gui.MSG_RECEIVED",
> new String[] { sourceMember.getName() });
> -
> + final String htmlContent;
> + if
(HTML_CONTENT_TYPE.equals(sourceMsg.getContentType()))
> + {
> + htmlContent = messageContent;
> + }
> + else
> + {
> + htmlContent =
StringEscapeUtils.escapeHtml4(messageContent);
> + }
> fireChatNotification(
> sourceChatRoom,
> INCOMING_MESSAGE,
> title,
> - messageContent,
> - evt.getMessage().getMessageUID());
> + htmlContent,
> + sourceMsg.getMessageUID());
> }
> }
> catch(Throwable t)
> @@ -1241,12 +1265,23 @@ public class NotificationManager
> "service.gui.MSG_RECEIVED",
> new String[]{evt.getSourceContact().getDisplayName()});
>
> + final Message sourceMsg = evt.getSourceMessage();
> + final String htmlContent;
> + if (HTML_CONTENT_TYPE.equals(sourceMsg.getContentType()))
> + {
> + htmlContent = sourceMsg.getContent();
> + }
> + else
> + {
> + htmlContent =
> +
StringEscapeUtils.escapeHtml4(sourceMsg.getContent());
> + }
> fireChatNotification(
> evt.getSourceContact(),
> INCOMING_MESSAGE,
> title,
> - evt.getSourceMessage().getContent(),
> - evt.getSourceMessage().getMessageUID());
> + htmlContent,
> + sourceMsg.getMessageUID());
> }
> catch(Throwable t)
> {
> diff --git
a/src/net/java/sip/communicator/plugin/notificationwiring/notificationwiring.manifest.mf
b/src/net/java/sip/communicator/plugin/notificationwiring/notificationwiring.manifest.mf
> index 46214fb…5f3a542 100644
> —
a/src/net/java/sip/communicator/plugin/notificationwiring/notificationwiring.manifest.mf
> +++
b/src/net/java/sip/communicator/plugin/notificationwiring/notificationwiring.manifest.mf
> @@ -17,5 +17,5 @@ Import-Package: javax.imageio,
> org.jitsi.service.neomedia.recording,
> org.jitsi.service.protocol.event,
> org.jitsi.service.resources,
> - org.osgi.framework
> -
> + org.osgi.framework,
> + org.apache.commons.lang3
>
>
> _______________________________________________
> commits mailing list
> commits@jitsi.org
> Unsubscribe instructions and other list options:
> http://lists.jitsi.org/mailman/listinfo/commits

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev


#3

Can you check if message type is indeed html when you get the &eacute; in

the

notification? (Because I don't think I intended to change anything in case

of

"normal" HTML messages.)

I'm just guessing here, but: the Swing Notifications probably need escaping
(due to the JLabel control), but Growl doesn't understand HTML at all.

Danny

Ingo


#4

Yes, it is plain/text. So I will just add unescaping back in growl?

···

On Mon, Feb 16, 2015 at 11:41 AM, Ingo Bauersachs <ingo@jitsi.org> wrote:

Can you check if message type is indeed html when you get the &eacute; in

the

notification? (Because I don't think I intended to change anything in case

of

"normal" HTML messages.)

I'm just guessing here, but: the Swing Notifications probably need escaping
(due to the JLabel control), but Growl doesn't understand HTML at all.

Danny

Ingo

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev


#5

Ideally, I think we need to move escaping up closer to the Swing component.
I have underestimated the impact it has at that location.

Is it possible to make the distinction between Swing notify and Growl, such
that only Swing gets escaped?

By the way, how does Growl handle HTML messages right now during
notification? It should show html chars in that case, does it do that?

···

On Mon, Feb 16, 2015 at 10:44 AM, Damian Minkov <damencho@jitsi.org> wrote:

Yes, it is plain/text. So I will just add unescaping back in growl?

On Mon, Feb 16, 2015 at 11:41 AM, Ingo Bauersachs <ingo@jitsi.org> wrote:
>> Can you check if message type is indeed html when you get the &eacute;
in
> the
>> notification? (Because I don't think I intended to change anything in
case
> of
>> "normal" HTML messages.)
>
> I'm just guessing here, but: the Swing Notifications probably need
escaping
> (due to the JLabel control), but Growl doesn't understand HTML at all.
>
>> Danny
>
> Ingo
>
>
> _______________________________________________
> dev mailing list
> dev@jitsi.org
> Unsubscribe instructions and other list options:
> http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev


#6

Yes, it is plain/text. So I will just add unescaping back in growl?

Well, that works for umlauts and accents, but what about actual HTML in a message? There would need to be a plaintext-version of an HTML-message I guess. Don't know if we have that. Danny knows that part better than me.

Ingo


#7

I'm quite sure I only receive message content and content type. That's why
I made the distinction in the first place.
I can check this evening to see how we should fix this. I don't have an
opportunity right now.

···

On Mon, Feb 16, 2015 at 11:01 AM, Ingo Bauersachs <ingo@jitsi.org> wrote:

> Yes, it is plain/text. So I will just add unescaping back in growl?

Well, that works for umlauts and accents, but what about actual HTML in a
message? There would need to be a plaintext-version of an HTML-message I
guess. Don't know if we have that. Danny knows that part better than me.

Ingo

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev


#8

Well growl is just removing the html stuff with a regexp, That's why
it fails to remove the &...; stuff.

// remove eventual HTML code before showing the pop-up message
messageBody = messageBody.replaceAll("</?\\w++[^>]*+>", "");
messageTitle = messageTitle.replaceAll("</?\\w++[^>]*+>", "");

Maybe we can do the escaping where needed ... I mean in popup
handlers, but on first look, there is no content type send to them :slight_smile:

I'm quite sure I only receive message content and content type. That's why I
made the distinction in the first place.
I can check this evening to see how we should fix this. I don't have an
opportunity right now.

No problem.

Thanks
damencho

···

On Mon, Feb 16, 2015 at 12:08 PM, Danny van Heumen <danny@dannyvanheumen.nl> wrote:

On Mon, Feb 16, 2015 at 11:01 AM, Ingo Bauersachs <ingo@jitsi.org> wrote:

> Yes, it is plain/text. So I will just add unescaping back in growl?

Well, that works for umlauts and accents, but what about actual HTML in a
message? There would need to be a plaintext-version of an HTML-message I
guess. Don't know if we have that. Danny knows that part better than me.

Ingo

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev


#9

Okay, well, that explains.

You are right, further "down" there's no content type anymore. That's why I
explicitly convert the plain text content to HTML with escaping, because it
assumes HTML down the line anyway. (Well, at least in some cases ...)

Danny

···

On Mon, Feb 16, 2015 at 11:18 AM, Damian Minkov <damencho@jitsi.org> wrote:

Well growl is just removing the html stuff with a regexp, That's why
it fails to remove the &...; stuff.

// remove eventual HTML code before showing the pop-up message
messageBody = messageBody.replaceAll("</?\\w++[^>]*+>", "");
messageTitle = messageTitle.replaceAll("</?\\w++[^>]*+>", "");

Maybe we can do the escaping where needed ... I mean in popup
handlers, but on first look, there is no content type send to them :slight_smile:

On Mon, Feb 16, 2015 at 12:08 PM, Danny van Heumen > <danny@dannyvanheumen.nl> wrote:
> I'm quite sure I only receive message content and content type. That's
why I
> made the distinction in the first place.
> I can check this evening to see how we should fix this. I don't have an
> opportunity right now.

No problem.

Thanks
damencho

>
> On Mon, Feb 16, 2015 at 11:01 AM, Ingo Bauersachs <ingo@jitsi.org> > wrote:
>>
>> > Yes, it is plain/text. So I will just add unescaping back in growl?
>>
>> Well, that works for umlauts and accents, but what about actual HTML in
a
>> message? There would need to be a plaintext-version of an HTML-message I
>> guess. Don't know if we have that. Danny knows that part better than me.
>>
>> Ingo
>>
>>
>> _______________________________________________
>> dev mailing list
>> dev@jitsi.org
>> Unsubscribe instructions and other list options:
>> http://lists.jitsi.org/mailman/listinfo/dev
>
>
>
> _______________________________________________
> dev mailing list
> dev@jitsi.org
> Unsubscribe instructions and other list options:
> http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev


#10

Oh, and for your last question.

I'd go with unescaping html entities *after* stripping html. Then you
should be good. HTML tags are already filtered and &entities; are easy to
convert, and we now know (because of my earlier patch) that *for sure* we
will be escaping plain text, so there's no chance to do unnecessary/double
unescaping.

···

On Mon, Feb 16, 2015 at 11:18 AM, Damian Minkov <damencho@jitsi.org> wrote:

Well growl is just removing the html stuff with a regexp, That's why
it fails to remove the &...; stuff.

// remove eventual HTML code before showing the pop-up message
messageBody = messageBody.replaceAll("</?\\w++[^>]*+>", "");
messageTitle = messageTitle.replaceAll("</?\\w++[^>]*+>", "");

Maybe we can do the escaping where needed ... I mean in popup
handlers, but on first look, there is no content type send to them :slight_smile:

On Mon, Feb 16, 2015 at 12:08 PM, Danny van Heumen > <danny@dannyvanheumen.nl> wrote:
> I'm quite sure I only receive message content and content type. That's
why I
> made the distinction in the first place.
> I can check this evening to see how we should fix this. I don't have an
> opportunity right now.

No problem.

Thanks
damencho

>
> On Mon, Feb 16, 2015 at 11:01 AM, Ingo Bauersachs <ingo@jitsi.org> > wrote:
>>
>> > Yes, it is plain/text. So I will just add unescaping back in growl?
>>
>> Well, that works for umlauts and accents, but what about actual HTML in
a
>> message? There would need to be a plaintext-version of an HTML-message I
>> guess. Don't know if we have that. Danny knows that part better than me.
>>
>> Ingo
>>
>>
>> _______________________________________________
>> dev mailing list
>> dev@jitsi.org
>> Unsubscribe instructions and other list options:
>> http://lists.jitsi.org/mailman/listinfo/dev
>
>
>
> _______________________________________________
> dev mailing list
> dev@jitsi.org
> Unsubscribe instructions and other list options:
> http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev


#11

Hi damencho,

You can safely assume that everything in the code that's "past" my
earlier patch will be in HTML. I can give you a hand if you like, but
you'd have to tell me which parts are causing you issues a.t.m. (I
didn't see any commits yet regarding this issue, so just in case ...)

Danny

···

On 16-02-15 11:18, Damian Minkov wrote:

Well growl is just removing the html stuff with a regexp, That's why
it fails to remove the &...; stuff.

// remove eventual HTML code before showing the pop-up message
messageBody = messageBody.replaceAll("</?\\w++[^>]*+>", "");
messageTitle = messageTitle.replaceAll("</?\\w++[^>]*+>", "");

Maybe we can do the escaping where needed ... I mean in popup
handlers, but on first look, there is no content type send to them :slight_smile:

On Mon, Feb 16, 2015 at 12:08 PM, Danny van Heumen > <danny@dannyvanheumen.nl> wrote:

I'm quite sure I only receive message content and content type. That's why I
made the distinction in the first place.
I can check this evening to see how we should fix this. I don't have an
opportunity right now.

No problem.

Thanks
damencho

On Mon, Feb 16, 2015 at 11:01 AM, Ingo Bauersachs <ingo@jitsi.org> wrote:

Yes, it is plain/text. So I will just add unescaping back in growl?

Well, that works for umlauts and accents, but what about actual HTML in a
message? There would need to be a plaintext-version of an HTML-message I
guess. Don't know if we have that. Danny knows that part better than me.

Ingo

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev


#12

Hi,

no there is no commit, I was asking for opinion what is the best way
to fix it. I can easily just add html unescape after removing html
tags in growl notification plugin.
Its just that we escape then unescape :slight_smile: I'll check it tomorrow how
hard it will be to add and the content type of the text when firing
the event, this way who needs it will escape the text. And will move
the escaping in popup notifications implementation. Just wondering
(now without checking) for which impl this is needed, for swing one
definitely, maybe and for windows what about the linux one ...

Regards
damencho

···

On Mon, Feb 16, 2015 at 9:21 PM, Danny van Heumen <danny@dannyvanheumen.nl> wrote:

Hi damencho,

You can safely assume that everything in the code that's "past" my
earlier patch will be in HTML. I can give you a hand if you like, but
you'd have to tell me which parts are causing you issues a.t.m. (I
didn't see any commits yet regarding this issue, so just in case ...)

Danny

On 16-02-15 11:18, Damian Minkov wrote:

Well growl is just removing the html stuff with a regexp, That's why
it fails to remove the &...; stuff.

// remove eventual HTML code before showing the pop-up message
messageBody = messageBody.replaceAll("</?\\w++[^>]*+>", "");
messageTitle = messageTitle.replaceAll("</?\\w++[^>]*+>", "");

Maybe we can do the escaping where needed ... I mean in popup
handlers, but on first look, there is no content type send to them :slight_smile:

On Mon, Feb 16, 2015 at 12:08 PM, Danny van Heumen >> <danny@dannyvanheumen.nl> wrote:

I'm quite sure I only receive message content and content type. That's why I
made the distinction in the first place.
I can check this evening to see how we should fix this. I don't have an
opportunity right now.

No problem.

Thanks
damencho

On Mon, Feb 16, 2015 at 11:01 AM, Ingo Bauersachs <ingo@jitsi.org> wrote:

Yes, it is plain/text. So I will just add unescaping back in growl?

Well, that works for umlauts and accents, but what about actual HTML in a
message? There would need to be a plaintext-version of an HTML-message I
guess. Don't know if we have that. Danny knows that part better than me.

Ingo

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev


#13

Windows uses Swing by default and I cannot imagine anyone using an alternative here.

Freundliche Grüsse,
Ingo Bauersachs

-- Sent from my tablet

···

On 16.02.2015, at 20:31, Damian Minkov <damencho@jitsi.org> wrote:

Hi,

no there is no commit, I was asking for opinion what is the best way
to fix it. I can easily just add html unescape after removing html
tags in growl notification plugin.
Its just that we escape then unescape :slight_smile: I'll check it tomorrow how
hard it will be to add and the content type of the text when firing
the event, this way who needs it will escape the text. And will move
the escaping in popup notifications implementation. Just wondering
(now without checking) for which impl this is needed, for swing one
definitely, maybe and for windows what about the linux one ...

Regards
damencho

On Mon, Feb 16, 2015 at 9:21 PM, Danny van Heumen > <danny@dannyvanheumen.nl> wrote:

Hi damencho,

You can safely assume that everything in the code that's "past" my
earlier patch will be in HTML. I can give you a hand if you like, but
you'd have to tell me which parts are causing you issues a.t.m. (I
didn't see any commits yet regarding this issue, so just in case ...)

Danny

On 16-02-15 11:18, Damian Minkov wrote:
Well growl is just removing the html stuff with a regexp, That's why
it fails to remove the &...; stuff.

// remove eventual HTML code before showing the pop-up message
messageBody = messageBody.replaceAll("</?\\w++[^>]*+>", "");
messageTitle = messageTitle.replaceAll("</?\\w++[^>]*+>", "");

Maybe we can do the escaping where needed ... I mean in popup
handlers, but on first look, there is no content type send to them :slight_smile:

On Mon, Feb 16, 2015 at 12:08 PM, Danny van Heumen >>> <danny@dannyvanheumen.nl> wrote:

I'm quite sure I only receive message content and content type. That's why I
made the distinction in the first place.
I can check this evening to see how we should fix this. I don't have an
opportunity right now.

No problem.

Thanks
damencho

On Mon, Feb 16, 2015 at 11:01 AM, Ingo Bauersachs <ingo@jitsi.org> wrote:

Yes, it is plain/text. So I will just add unescaping back in growl?

Well, that works for umlauts and accents, but what about actual HTML in a
message? There would need to be a plaintext-version of an HTML-message I
guess. Don't know if we have that. Danny knows that part better than me.

Ingo

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev


#14

Hmm.. I only know about the Swing one for sure.

I agree that it seems a bit senseless to first escape+htmlify and then
undo that. That is, unless you want to make html the standard and you
treat Growl as the exception. The point in code where my patch is, is
where you lose the content type. That particular method receives content
+ type, so you could include both in further processing and handle each
type appropriately.

The unfortunate thing is that you would still need to get rid of html
and unescape in case of Growl, because you can still receive 'text/html'
messages and that cannot be avoided.

Danny

···

On 16-02-15 20:30, Damian Minkov wrote:

Hi,

no there is no commit, I was asking for opinion what is the best way
to fix it. I can easily just add html unescape after removing html
tags in growl notification plugin.
Its just that we escape then unescape :slight_smile: I'll check it tomorrow how
hard it will be to add and the content type of the text when firing
the event, this way who needs it will escape the text. And will move
the escaping in popup notifications implementation. Just wondering
(now without checking) for which impl this is needed, for swing one
definitely, maybe and for windows what about the linux one ...

Regards
damencho

On Mon, Feb 16, 2015 at 9:21 PM, Danny van Heumen > <danny@dannyvanheumen.nl> wrote:

Hi damencho,

You can safely assume that everything in the code that's "past" my
earlier patch will be in HTML. I can give you a hand if you like, but
you'd have to tell me which parts are causing you issues a.t.m. (I
didn't see any commits yet regarding this issue, so just in case ...)

Danny

On 16-02-15 11:18, Damian Minkov wrote:

Well growl is just removing the html stuff with a regexp, That's why
it fails to remove the &...; stuff.

// remove eventual HTML code before showing the pop-up message
messageBody = messageBody.replaceAll("</?\\w++[^>]*+>", "");
messageTitle = messageTitle.replaceAll("</?\\w++[^>]*+>", "");

Maybe we can do the escaping where needed ... I mean in popup
handlers, but on first look, there is no content type send to them :slight_smile:

On Mon, Feb 16, 2015 at 12:08 PM, Danny van Heumen >>> <danny@dannyvanheumen.nl> wrote:

I'm quite sure I only receive message content and content type. That's why I
made the distinction in the first place.
I can check this evening to see how we should fix this. I don't have an
opportunity right now.

No problem.

Thanks
damencho

On Mon, Feb 16, 2015 at 11:01 AM, Ingo Bauersachs <ingo@jitsi.org> wrote:

Yes, it is plain/text. So I will just add unescaping back in growl?

Well, that works for umlauts and accents, but what about actual HTML in a
message? There would need to be a plaintext-version of an HTML-message I
guess. Don't know if we have that. Danny knows that part better than me.

Ingo

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev