[Rt-commit] rt branch 5.0/quoting-text-with-ckeditor created. rt-5.0.4-20-g71155c9704

BPS Git Server git at git.bestpractical.com
Sun Jun 11 19:28:07 UTC 2023


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "rt".

The branch, 5.0/quoting-text-with-ckeditor has been created
        at  71155c9704f1887876cdab8886f40bda9ee88a28 (commit)

- Log -----------------------------------------------------------------
commit 71155c9704f1887876cdab8886f40bda9ee88a28
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Thu Jun 1 23:57:44 2023 +0300

    Improve replying with selected part
    
    * Use JS to generate either html or text depending on RT config
    
    * Pass QuoteContent and QuoteContentType to the server
    
    * Let the server deal with quoting and converting type if needed
    
    * Write a few tests around a new function

diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index 9f0909294a..fd6843dbbe 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -325,25 +325,37 @@ sub HasContent {
 }
 
 
-
 =head2 Content PARAMHASH
 
-If this transaction has attached mime objects, returns the body of the first
-textual part (as defined in RT::I18N::IsTextualContentType).  Otherwise,
-returns the message "This transaction appears to have no content".
+Returns the content of this transaction as a string. Optionally quoted. When transaction
+has no content, returns the message "This transaction appears to have no content". Read
+L<ContentObj> to understand how the content is determined.
+
+Arguments:
+
+=over 4
+
+=item Type - type of the content to return, either C<text/plain> or C<text/html>
+
+=item Quote - quote the content and prepends it with the L<QuoteHeader>
+
+=item Wrap - size at which to wrap the content in case of C<text/plain>. 
+
+=back
+
+Examples:
 
-Takes a paramhash.  If the $args{'Quote'} parameter is set, wraps this message
-at $args{'Wrap'}.  $args{'Wrap'} is set from the $QuoteWrapWidth
-config variable.
+    # returns html content of the transaction
+    my $content = $txn->Content( Type => 'text/html' );
 
-If $args{'Type'} is set to C<text/html>, this will return an HTML 
-part of the message, if available.  Otherwise it looks for a text/plain
-part. If $args{'Type'} is missing, it defaults to the value of 
-C<$RT::Transaction::PreferredContentType>, if that's missing too, 
-defaults to textual.
+    # returns plain text content of the transaction
+    my $content = $txn->Content( Type => 'text/plain' );
 
-All the MIME attachments are excluded here, because it doesn't make much sense
-to use them as transaction content.
+    # returns html content of the transaction, quoted
+    my $content = $txn->Content( Type => 'text/html', Quote => 1 );
+
+    # returns text content of the transaction, quoted and wrapped at 60 characters
+    my $content = $txn->Content( Type => 'text/plain', Quote => 1, Wrap => 60 );
 
 =cut
 
@@ -352,70 +364,155 @@ sub Content {
     my %args = (
         Type => $PreferredContentType || '',
         Quote => 0,
-        Wrap => RT->Config->Get('QuoteWrapWidth') || 70,
         @_
     );
 
-    my $content;
-    if ( my $content_obj = 
-        $self->ContentObj( $args{Type} ? ( Type => $args{Type}) : () ) )
-    {
+    my ($content, $content_type);
+    if ( my $content_obj = $self->ContentObj( $args{Type} ? ( Type => $args{Type}) : () ) ) {
         $content = $content_obj->Content ||'';
+        $content_type = lc $content_obj->ContentType;
 
-        if ( lc $content_obj->ContentType eq 'text/html' ) {
-            $content =~ s/(?:(<\/div>)|<p>|<br\s*\/?>|<div(\s+class="[^"]+")?>)\s*--\s+<br\s*\/?>.*?$/$1/s if $args{'Quote'};
-
-            if ($args{Type} ne 'text/html') {
-                $content = RT::Interface::Email::ConvertHTMLToText($content);
-            } else {
-                # Scrub out <html>, <head>, <meta>, and <body>, and
-                # leave all else untouched.
-                my $scrubber = HTML::Scrubber->new();
-                $scrubber->rules(
-                    html => 0,
-                    head => 0,
-                    meta => 0,
-                    body => 0,
-                );
-                $scrubber->default( 1 => { '*' => 1 } );
-                $content = $scrubber->scrub( $content );
+        # remove signature
+        if ( $args{'Quote'} ) {
+            if ( $content_type eq 'text/html' ) {
+                $content =~ s/(?:(<\/div>)|<p>|<br\s*\/?>|<div(\s+class="[^"]+")?>)\s*--\s+<br\s*\/?>.*?$/$1/s;
             }
-        }
-        else {
-            $content =~ s/\n-- \n.*?$//s if $args{'Quote'};
-            if ($args{Type} eq 'text/html') {
-                # Extremely simple text->html converter
-                $content =~ s/&/&/g;
-                $content =~ s/</</g;
-                $content =~ s/>/>/g;
-                $content = qq|<pre style="white-space: pre-wrap; font-family: monospace;">$content</pre>|;
+            else {
+                $content =~ s/\n-- \n.*?$//s;
             }
         }
     }
+    else { # If all else fails, return a message that we couldn't find any content
+        $content = $self->loc('This transaction appears to have no content');
+        $content_type = 'text/plain';
+    }
+
+    $content = $self->QuoteContent(
+        %args,
+        Content => $content,
+        ContentType => $content_type,
+        $args{Quote}? (QuoteHeader => $self->QuoteHeader): (),
+    );
+
+    return ($content);
+}
+
+
+=head2 QuoteContent
+
+B<Class method> utility. Takes content, its type, options and returns the content,
+quoted, wrapped and with header if aked.
+
+Takes a paramhash.
+
+=over 4
+
+=item Type - expected content type that should be returned, either 'text/html' or 'text/plain' (default)
+
+=item Content - the content to work on
+
+=item ContentType - content type of the content, either 'text/html' or 'text/plain'
+
+=item Quote - boolean, whether to quote the content or not
+
+=item QuoteHeader - string, header to prepend to the content if C<Quote> is true
+
+=item Wrap - integer, wrap the content at this many characters, defaults to C<QuoteWrapWidth>
+option from RT config or 70 if it's not set. Applies B<only> if C<Quote> is true and if expected
+content type is C<text/plain>.
+
+=back
+
+Example:
 
-    # If all else fails, return a message that we couldn't find any content
+    # quote some html content as text/html
+    my $res = RT::Transaction->QuoteContent(
+        Type => 'text/html',
+        Content => '<p>Hello, world!</p>',
+        ContentType => 'text/html',
+        Quote => 1,
+        QuoteHeader => 'John Doe wrote:',
+    );
+
+=cut
+
+sub QuoteContent {
+    my $self = shift;
+    my %args = (
+        Type => $PreferredContentType || '',
+
+        Content => '',
+        ContentType => '',
+
+        Quote => 0,
+        QuoteHeader => '',
+        Wrap => RT->Config->Get('QuoteWrapWidth') || 70,
+
+        @_
+    );
+
+    my $expected_type = lc( $args{Type} || '' );
+    my $content       = $args{Content} || '';
+    my $content_type  = lc( $args{ContentType} || '' );
+
+    return '' unless $content;
+
+    if ( $content_type eq 'text/html' ) {
+        if ($expected_type ne 'text/html') {
+            $content = RT::Interface::Email::ConvertHTMLToText($content);
+        } else {
+            # Scrub out <html>, <head>, <meta>, and <body>, and
+            # leave all else untouched.
+            my $scrubber = HTML::Scrubber->new();
+            $scrubber->rules(
+                html => 0,
+                head => 0,
+                meta => 0,
+                body => 0,
+            );
+            $scrubber->default( 1 => { '*' => 1 } );
+            $content = $scrubber->scrub( $content );
+        }
+    }
     else {
-        $content = $self->loc('This transaction appears to have no content');
+        if ($expected_type eq 'text/html') {
+            # Extremely simple text->html converter
+            $content =~ s/&/&/g;
+            $content =~ s/</</g;
+            $content =~ s/>/>/g;
+            $content = qq|<pre style="white-space: pre-wrap; font-family: monospace;">$content</pre>|;
+        }
     }
 
     if ( $args{'Quote'} ) {
-        if ($args{Type} eq 'text/html') {
-            $content = '<div class="gmail_quote">'
-                . $self->QuoteHeader
-                . '<br /><blockquote class="gmail_quote" type="cite">'
+        if ( $expected_type eq 'text/html' ) {
+            $content = 
+                '<blockquote class="gmail_quote" type="cite">'
                 . $content
-                . '</blockquote></div>';
+                . '</blockquote>';
         } else {
-            $content = $self->ApplyQuoteWrap(content => $content,
-                                             cols    => $args{'Wrap'} );
-
-            $content = $self->QuoteHeader . "\n$content";
+            $content = $self->ApplyQuoteWrap(
+                content => $content,
+                cols    => $args{'Wrap'} 
+            );
+        }
+    }
+    
+    if ( $args{'Quote'} && $args{'QuoteHeader'} ) {
+        if ($args{Type} eq 'text/html') {
+            $content =
+                '<div class="gmail_quote">'
+                . $args{'QuoteHeader'} .'<br />'. $content
+                . '</div>';
+        } else {
+            $content = $args{'QuoteHeader'} . "\n". $content;
         }
     }
 
     return ($content);
 }
 
+
 =head2 QuoteHeader
 
 Returns text prepended to content when transaction is quoted
@@ -532,7 +629,21 @@ sub Addresses {
 
 =head2 ContentObj 
 
-Returns the RT::Attachment object which contains the content for this Transaction
+Returns the L<RT::Attachment> object which contains the content for this Transaction.
+
+Takes C<Type> argument, which can be either C<text/plain> or C<text/html>, and defines
+preferred content type. If C<Type> is set to C<text/html>, this will return an HTML 
+part of the message, if available. Otherwise it looks for a C<text/plain>
+part. If the argument is missing, it defaults to the value of 
+C<$RT::Transaction::PreferredContentType>.
+
+If there is no attachement of the preferred content type, returns the first textual part
+(as defined in L<RT::I18N::IsTextualContentType>).
+
+All the attachments with filenames or marked as attached files are excluded here,
+because it doesn't make much sense to use them as transaction content.
+
+Returns undef if there is no attachment matching these rules.
 
 =cut
 
diff --git a/share/html/Elements/MessageBox b/share/html/Elements/MessageBox
index b5c6677348..8d2e928d6d 100644
--- a/share/html/Elements/MessageBox
+++ b/share/html/Elements/MessageBox
@@ -80,7 +80,26 @@ my $message = '';
 if ( $QuoteTransaction ) {
     my $transaction = RT::Transaction->new( $session{'CurrentUser'} );
     $transaction->Load( $QuoteTransaction );
-    $message = $transaction->Content( Quote => 1, Type  => $Type );
+
+    if ( $transaction->Id && !$QuoteContent ) {
+        $message = $transaction->Content( Quote => 1, Type  => $Type );
+    } else {
+        $message = RT::Transaction->QuoteContent(
+            Quote => 1,
+            Type  => $Type,
+            Content => $QuoteContent,
+            ContentType => $QuoteContentType,
+            $transaction->Id? (QuoteHeader => $transaction->QuoteHeader) : (),
+        );
+    }
+}
+elsif ( $QuoteContent ) {
+    $message = RT::Transaction->QuoteContent(
+        Type => $Type,
+        Quote => 1,
+        Content => $QuoteContent,
+        ContentType => $QuoteContentType,
+    );
 }
 
 my $signature = $session{'CurrentUser'}->UserObj->Signature // "";
@@ -145,6 +164,8 @@ if ($Width) {
 </%INIT>
 <%ARGS>
 $QuoteTransaction          => undef
+$QuoteContent              => undef
+$QuoteContentType          => undef
 $Name                      => 'Content'
 $Default                   => ''
 $Width                     => RT->Config->Get('MessageBoxWidth', $session{'CurrentUser'} )
diff --git a/share/static/js/quoteselection.js b/share/static/js/quoteselection.js
index 48e01a7557..fca08a265f 100644
--- a/share/static/js/quoteselection.js
+++ b/share/static/js/quoteselection.js
@@ -1,79 +1,118 @@
 jQuery(function() {
-    if(RT.Config.QuoteSelectedText) {
-        var reply_from_selection = function(ev) {
-            var link = jQuery(this);
-
-            var selection;
-            var activeElement;
-            if (window.getSelection) {
-                selection = window.getSelection();
-            } else {
-                return;
-            }
+    if(!RT.Config.QuoteSelectedText) {
+        return;
+    }
 
-            if (selection.rangeCount) {
-                activeElement = selection.getRangeAt(0);
-            } else {
-                return;
+    var add_sub_container = function (container, tagName) {
+        var sub = document.createElement(tagName);
+        container.appendChild(sub);
+        return sub;
+    };
+
+    var range_html = function (range) {
+        var topContainer = document.createElement('div');
+        var container = topContainer;
+
+        var fragment = range.cloneContents();
+
+        var child = fragment.firstElementChild;
+        if (child) {
+            var tn = child.tagName
+            if (tn == "LI") {
+                container = add_sub_container(container, 'ul');
+            } else if (tn == "DT" || tn == "DD") {
+                container = add_sub_container(container, 'dl');
+            } else if (tn == "TD" || tn == "TH") {
+                container = add_sub_container(container, 'table');
+                container = add_sub_container(container, 'tbody');
+                container = add_sub_container(container, 'tr');
+            } else if (tn == "TR") {
+                container = add_sub_container(container, 'table');
+                container = add_sub_container(container, 'tbody');
+            } else if (tn == "TBODY" || tn == "THEAD" || tn == "TFOOT") {
+                container = add_sub_container(container, 'table');
             }
+        }
 
-            // check if selection has commonAncestorContainer with class 'messagebody'
-            var commonAncestor = activeElement.commonAncestorContainer;
-            if (commonAncestor) {
-                var isMessageBody = false;
-                var parent = commonAncestor.parentNode;
-                while (parent) {
-                    if (parent.className && parent.className.indexOf('messagebody') != -1) {
-                        isMessageBody = true;
-                        break;
-                    }
-                    parent = parent.parentNode;
-                }
-                if (!isMessageBody) {
-                    return;
+        container.appendChild(fragment);
+        return topContainer.innerHTML;
+    };
+
+    var reply_from_selection = function(ev) {
+        var link = jQuery(this);
+
+        var selection;
+        var activeElement;
+        if (window.getSelection) {
+            selection = window.getSelection();
+        } else {
+            return;
+        }
+
+        if (selection.rangeCount) {
+            activeElement = selection.getRangeAt(0);
+        } else {
+            return;
+        }
+
+        // check if selection has commonAncestorContainer with class 'messagebody'
+        var commonAncestor = activeElement.commonAncestorContainer;
+        if (commonAncestor) {
+            var isMessageBody = false;
+            var parent = commonAncestor.parentNode;
+            while (parent) {
+                if (parent.className && parent.className.indexOf('messagebody') != -1) {
+                    isMessageBody = true;
+                    break;
                 }
+                parent = parent.parentNode;
+            }
+            if (!isMessageBody) {
+                return;
             }
+        }
 
+        var ct = '';
+        if ( RT.Config.MessageBoxRichText ) {
+            selection = range_html(activeElement);
+            ct = 'text/html';
+        }
+        else {
             if (selection.toString)
                 selection = selection.toString();
 
-            if (typeof(selection) !== "string" || selection.length < 3)
-                return;
+            selection = selection.concat("\n\n");
+            ct = 'text/plain';
+        }
+        if (typeof(selection) !== "string" || selection.length < 3)
+            return;
 
-            // TODO: wrap long lines before quoting
-            selection = selection.replace(/^/gm, "> ");
-            if ( RT.Config.MessageBoxRichText ) {
-                selection = selection.replace(/\r?\n/g, "<br>");
-                selection = selection.concat("<br><br>");
-            }
-            else {
-                selection = selection.concat("\n\n");
-            }
-            selection = encodeURIComponent(selection);
+        selection = encodeURIComponent(selection);
+        ct = encodeURIComponent(ct);
 
-            if ( !link.prop('data-href') ) {
-                link.prop('data-href', link.attr('href'));
-            }
-            link.attr("href", link.prop("data-href").concat("&UpdateContent=" + selection));
-        };
+        if ( !link.prop('data-href') ) {
+            link.prop('data-href', link.attr('href'));
+        }
+        link.attr("href", link.prop("data-href")
+            .concat("&QuoteContent=" + selection + "&QuoteContentType=" + ct));
+    };
 
-        var apply_quote = function() {
-            var link = jQuery(this);
-            if (link.data("quote-selection"))
-                return;
-            link.data("quote-selection",true);
-            link.click(reply_from_selection);
-        };
-
-        jQuery(
-            ".reply-link, "         +
-            ".comment-link, "       +
-            "#page-actions-reply, " +
-            "#page-actions-comment"
-        ).each(apply_quote);
-
-        jQuery(document).ajaxComplete(function(ev){
-            jQuery(".reply-link, .comment-link").each(apply_quote);
-        });
-    }
+    var apply_quote = function() {
+        var link = jQuery(this);
+        if (link.data("quote-selection"))
+            return;
+        link.data("quote-selection",true);
+        link.click(reply_from_selection);
+    };
+
+    jQuery(
+        ".reply-link, "         +
+        ".comment-link, "       +
+        "#page-actions-reply, " +
+        "#page-actions-comment"
+    ).each(apply_quote);
+
+    jQuery(document).ajaxComplete(function(ev){
+        jQuery(".reply-link, .comment-link").each(apply_quote);
+    });
 });
diff --git a/t/transaction/content.t b/t/transaction/content.t
new file mode 100644
index 0000000000..d74cb45deb
--- /dev/null
+++ b/t/transaction/content.t
@@ -0,0 +1,93 @@
+use strict;
+use warnings;
+
+use RT::Test tests => undef;
+
+# test RT::Transaction->QuoteContent
+{
+    {
+        my $got = RT::Transaction->QuoteContent(
+            Type => 'text/plain',
+            Quote => 1,
+            Content => 'foo',
+            ContentType => 'text/plain',
+        );
+        is $got, "> foo", "ok";
+    }
+
+    {
+        my $got = RT::Transaction->QuoteContent(
+            Type => 'text/html',
+            Quote => 1,
+            Content => '< jane & joe >',
+            ContentType => 'text/plain',
+        );
+        is $got,
+            '<blockquote class="gmail_quote" type="cite">'
+            .'<pre style="white-space: pre-wrap; font-family: monospace;">'
+            .'< jane & joe >'
+            .'</pre></blockquote>',
+            "ok",
+        ;
+    }
+
+    {
+        my $got = RT::Transaction->QuoteContent(
+            Type => 'text/html',
+            Quote => 1,
+            Content => '<stron>jane & joe</strong>',
+            ContentType => 'text/html',
+        );
+        is $got,
+            '<blockquote class="gmail_quote" type="cite">'
+            .'<stron>jane & joe</strong>'
+            .'</blockquote>',
+            "ok",
+        ;
+    }
+
+    {
+        my $got = RT::Transaction->QuoteContent(
+            Type => 'text/plain',
+            Quote => 1,
+            Content => '<stron>jane & joe</strong>',
+            ContentType => 'text/html',
+        );
+        is $got,
+            "> jane & joe\n",
+            "ok",
+        ;
+    }
+
+    {
+        my $got = RT::Transaction->QuoteContent(
+            Type => 'text/html',
+            Quote => 1,
+            QuoteHeader => 'Nemo wrote:',
+            Content => '<stron>jane & joe</strong>',
+            ContentType => 'text/html',
+        );
+        is $got,
+            '<div class="gmail_quote">Nemo wrote:<br />'
+            .'<blockquote class="gmail_quote" type="cite">'
+            .'<stron>jane & joe</strong>'
+            .'</blockquote>'
+            .'</div>',
+            "ok",
+        ;
+    }
+
+    {
+        my $got = RT::Transaction->QuoteContent(
+            Type => 'text/plain',
+            Quote => 1,
+            QuoteHeader => 'Nemo wrote:',
+            Content => 'foo',
+            ContentType => 'text/plain',
+        );
+        is $got, "Nemo wrote:\n> foo", "ok";
+    }
+
+}
+
+done_testing();
\ No newline at end of file

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


hooks/post-receive
-- 
rt


More information about the rt-commit mailing list