[Rt-commit] rt branch, 4.2/showhistory-paths-and-titlebars, created. rt-4.2.4-27-gafbf0ab

Kevin Falcone falcone at bestpractical.com
Tue May 27 11:01:21 EDT 2014


The branch, 4.2/showhistory-paths-and-titlebars has been created
        at  afbf0aba932d41b6b46a8677782fe9b1c176444f (commit)

- Log -----------------------------------------------------------------
commit 47bf64f67250f673c2f6788ffe9a355a12d5b891
Author: Kevin Falcone <falcone at bestpractical.com>
Date:   Fri May 23 17:31:30 2014 -0400

    Assuming relative paths in ShowHistory breaks Approvals history display
    
    Show Outgoing Email goes to /Approvals/ShowEmailRecord.html which
    doesn't exist.  Additionally, Reply/Comment/Forward are broken (although
    shouldn't be visible, fixed in a later commit).
    
    This also shows up in RTIR where links point to
    /RTIR/ShowEmailRecord.html and /RTIR/Attachments/
    
    Fixes issues 29800 and lays groundwork for a fix in RTIR for 29775.
    
    While you could list /Ticket/Update.html and /Ticket/Crypt.html for the
    6 *Path variables passed in %ARGS, it's nicer if you can just say "use
    everything from /Ticket/, like you used to in 4.0".
    
    Broken when moving to relative-by-default paths in 8274e2b6.
    
    I don't think you can jam something malicious into PathPrefix since
    /Elements/ShowHistory isn't directly callable and the JS helper doesn't
    pass extra arguments, and I don't see other callsites passing %ARGS to
    ShowHistory.

diff --git a/share/html/Approvals/Display.html b/share/html/Approvals/Display.html
index bb1810c..b0f0251 100644
--- a/share/html/Approvals/Display.html
+++ b/share/html/Approvals/Display.html
@@ -50,7 +50,7 @@
 <form method="post" action="<%RT->Config->Get('WebPath')%>/Approvals/index.html">
 
 <&| /Widgets/TitleBox, title => $title &>
-<& /Elements/ShowHistory , Object => $Ticket, ShowTitle => 0, ShowHeaders => 0, ShowDisplayModes => 0, ShowTitleBarCommands => 0 &>
+<& /Elements/ShowHistory , Object => $Ticket, ShowTitle => 0, ShowHeaders => 0, ShowDisplayModes => 0, ShowTitleBarCommands => 0, PathPrefix => RT->Config->Get('WebPath')."/Ticket/" &>
 <hr />
 <& Elements/Approve, ticket => $Ticket, ShowApproving => 0 &>
 </&>
diff --git a/share/html/Approvals/Elements/Approve b/share/html/Approvals/Elements/Approve
index 5c32f2c..860ea64 100644
--- a/share/html/Approvals/Elements/Approve
+++ b/share/html/Approvals/Elements/Approve
@@ -58,7 +58,7 @@
       <& /Ticket/Elements/ShowCustomFields, Ticket => $approving &>
 % }
 % if ($ShowHistory) {
-      <& /Elements/ShowHistory, Object => $approving, ShowTitle => 0, ShowHeaders => 0, ShowDisplayModes => 0, ShowTitleBarCommands => 0 &>
+      <& /Elements/ShowHistory, Object => $approving, ShowTitle => 0, ShowHeaders => 0, ShowDisplayModes => 0, ShowTitleBarCommands => 0, PathPrefix => RT->Config->Get('WebPath')."/Ticket/" &>
 % }
     </div>
   </div>
diff --git a/share/html/Elements/ShowHistory b/share/html/Elements/ShowHistory
index c86058b..b05e080 100644
--- a/share/html/Elements/ShowHistory
+++ b/share/html/Elements/ShowHistory
@@ -165,9 +165,10 @@ for my $attachment (@{$Attachments->ItemsArrayRef()}) {
         EncryptionPath  => 'Crypt.html',
     );
 
+    my $prefix = $ARGS{PathPrefix}||'';
     while ( my ($arg, $path) = each %tmp ) {
         next if defined $ARGS{ $arg };
-        $ARGS{ $arg } = $path;
+        $ARGS{ $arg } = $prefix.$path;
     }
 }
 
@@ -185,4 +186,6 @@ $AttachmentContent => $Object->TextAttachments
 $ShowHeaders       => 0
 $ShowTitle         => 1
 $ShowDisplayModes  => 1
+
+$PathPrefix        => ''
 </%ARGS>

commit bb8625760ce3635ed8d8bd3e6f6d74dfae584971
Author: Kevin Falcone <falcone at bestpractical.com>
Date:   Fri May 23 17:39:16 2014 -0400

    In 4.0, ShowTitleBarCommands didn't hide Outgoing Email links
    
    The 4.2 equivalent ShowActions should be compatible for the places
    that want to suppress Reply/Comment/Forward.
    
    This makes the code change, leaves whitespace terrible.

diff --git a/share/html/Elements/ShowTransaction b/share/html/Elements/ShowTransaction
index e60a79e..2c98005 100644
--- a/share/html/Elements/ShowTransaction
+++ b/share/html/Elements/ShowTransaction
@@ -144,7 +144,6 @@ if ( $ShowBody && !$Attachments ) {
 }
 
 my @actions = ();
-if ( $ShowActions ) {
     my $txn_type = $Transaction->Type;
     if ( $txn_type =~ /EmailRecord$/ ) {
         push @actions, {
@@ -160,7 +159,7 @@ if ( $ShowActions ) {
     }
 
     # If the transaction has anything attached to it at all
-    elsif ( %$Attachments ) {
+    elsif ( %$Attachments && $ShowActions ) {
         my %has_right = map {
             $_ => RT::ACE->CanonicalizeRightName( $_ . $record_type )
         } qw(Modify CommentOn ReplyTo);
@@ -225,7 +224,6 @@ if ( $ShowActions ) {
             };
         }
     }
-}
 
 $m->callback(
     %ARGS,

commit 5f78709d6646961a83016f8d0ad1e77b4c3d8b65
Author: Kevin Falcone <falcone at bestpractical.com>
Date:   Fri May 23 17:41:09 2014 -0400

    Whitespace only change
    
    The previous commit removed an outer if block and pushed it into the
    boolean check of the elsif, leaving it weirdly indented.  This left
    shifts everything as a separate commit to make reading the previous code
    change clearer.

diff --git a/share/html/Elements/ShowTransaction b/share/html/Elements/ShowTransaction
index 2c98005..e28be7e 100644
--- a/share/html/Elements/ShowTransaction
+++ b/share/html/Elements/ShowTransaction
@@ -144,86 +144,86 @@ if ( $ShowBody && !$Attachments ) {
 }
 
 my @actions = ();
-    my $txn_type = $Transaction->Type;
-    if ( $txn_type =~ /EmailRecord$/ ) {
-        push @actions, {
-            title  => loc('Show'),
-            target => '_blank',
-            path   => $EmailRecordPath
-                .'?id='. $Object->id
-                .'&Transaction='. $Transaction->id
-                .'&Attachment='. ( $Attachments->{0}[0] && $Attachments->{0}[0]->id ),
-        } if $EmailRecordPath;
+my $txn_type = $Transaction->Type;
+if ( $txn_type =~ /EmailRecord$/ ) {
+    push @actions, {
+        title  => loc('Show'),
+        target => '_blank',
+        path   => $EmailRecordPath
+            .'?id='. $Object->id
+            .'&Transaction='. $Transaction->id
+            .'&Attachment='. ( $Attachments->{0}[0] && $Attachments->{0}[0]->id ),
+    } if $EmailRecordPath;
 
-        $ShowBody = 0;
-    }
+    $ShowBody = 0;
+}
 
-    # If the transaction has anything attached to it at all
-    elsif ( %$Attachments && $ShowActions ) {
-        my %has_right = map {
-            $_ => RT::ACE->CanonicalizeRightName( $_ . $record_type )
-        } qw(Modify CommentOn ReplyTo);
-        $has_right{'Forward'} = RT::ACE->CanonicalizeRightName('ForwardMessage');
+# If the transaction has anything attached to it at all
+elsif ( %$Attachments && $ShowActions ) {
+    my %has_right = map {
+        $_ => RT::ACE->CanonicalizeRightName( $_ . $record_type )
+    } qw(Modify CommentOn ReplyTo);
+    $has_right{'Forward'} = RT::ACE->CanonicalizeRightName('ForwardMessage');
 
-        my $can_modify = $has_right{'Modify'}
-            && $Object->CurrentUserHasRight( $has_right{'Modify'} );
+    my $can_modify = $has_right{'Modify'}
+        && $Object->CurrentUserHasRight( $has_right{'Modify'} );
 
-        if ( $UpdatePath && $has_right{'ReplyTo'}
-            && ( $can_modify
-                || $Object->CurrentUserHasRight( $has_right{'ReplyTo'} )
-            )
-        ) {
-            push @actions, {
-                class  => "reply-link",
-                title  => loc('Reply'),
-                path   => $UpdatePath
-                    .'?id='. $Object->id
-                    .'&QuoteTransaction='. $Transaction->id
-                    .'&Action=Respond'
-                ,
-            };
-        }
-        if ( $UpdatePath && $has_right{'CommentOn'}
-            && ( $can_modify
-                || $Object->CurrentUserHasRight( $has_right{'CommentOn'} )
-            )
-        ) {
-            push @actions, {
-                class  => "comment-link",
-                title  => loc('Comment'),
-                path   => $UpdatePath
-                    .'?id='. $Object->id
-                    .'&QuoteTransaction='. $Transaction->id
-                    .'&Action=Comment'
-                ,
-            };
-        }
-        if ( $ForwardPath && $has_right{'Forward'}
-            && $Object->CurrentUserHasRight( $has_right{'Forward'} )
-        ) {
-            push @actions, {
-                class  => "forward-link",
-                title  => loc('Forward'),
-                path   => $ForwardPath
-                    .'?id='. $Object->id
-                    .'&QuoteTransaction='. $Transaction->id
-                ,
-            };
-        }
-        if ( $EncryptionPath && $can_modify
-            && RT->Config->Get('Crypt')->{'Enable'}
-            && RT->Config->Get('Crypt')->{'AllowEncryptDataInDB'}
-        ) {
-            push @actions, {
-                class  => "encryption-link",
-                title  => loc('Encrypt/Decrypt'),
-                path   => $EncryptionPath
-                    .'?id='. $Transaction->id
-                    .'&QuoteTransaction='. $Transaction->id
-                ,
-            };
-        }
+    if ( $UpdatePath && $has_right{'ReplyTo'}
+        && ( $can_modify
+            || $Object->CurrentUserHasRight( $has_right{'ReplyTo'} )
+        )
+    ) {
+        push @actions, {
+            class  => "reply-link",
+            title  => loc('Reply'),
+            path   => $UpdatePath
+                .'?id='. $Object->id
+                .'&QuoteTransaction='. $Transaction->id
+                .'&Action=Respond'
+            ,
+        };
     }
+    if ( $UpdatePath && $has_right{'CommentOn'}
+        && ( $can_modify
+            || $Object->CurrentUserHasRight( $has_right{'CommentOn'} )
+        )
+    ) {
+        push @actions, {
+            class  => "comment-link",
+            title  => loc('Comment'),
+            path   => $UpdatePath
+                .'?id='. $Object->id
+                .'&QuoteTransaction='. $Transaction->id
+                .'&Action=Comment'
+            ,
+        };
+    }
+    if ( $ForwardPath && $has_right{'Forward'}
+        && $Object->CurrentUserHasRight( $has_right{'Forward'} )
+    ) {
+        push @actions, {
+            class  => "forward-link",
+            title  => loc('Forward'),
+            path   => $ForwardPath
+                .'?id='. $Object->id
+                .'&QuoteTransaction='. $Transaction->id
+            ,
+        };
+    }
+    if ( $EncryptionPath && $can_modify
+        && RT->Config->Get('Crypt')->{'Enable'}
+        && RT->Config->Get('Crypt')->{'AllowEncryptDataInDB'}
+    ) {
+        push @actions, {
+            class  => "encryption-link",
+            title  => loc('Encrypt/Decrypt'),
+            path   => $EncryptionPath
+                .'?id='. $Transaction->id
+                .'&QuoteTransaction='. $Transaction->id
+            ,
+        };
+    }
+}
 
 $m->callback(
     %ARGS,

commit afbf0aba932d41b6b46a8677782fe9b1c176444f
Author: Kevin Falcone <falcone at bestpractical.com>
Date:   Fri May 23 18:00:16 2014 -0400

    Switch to ShowActions from ShowTitleBarCommands
    
    f0137c97 deleted ShowTitleBarCommands but commits which were merged
    later (0d8eacf3 and 775f4bd1) used them for newly refactored code,
    causing Reply/Comment/Forward to pop up all over the Approvals display,
    and generating links that either loop back to the approvals page or
    otherwise don't work.

diff --git a/share/html/Approvals/Display.html b/share/html/Approvals/Display.html
index b0f0251..efa46ef 100644
--- a/share/html/Approvals/Display.html
+++ b/share/html/Approvals/Display.html
@@ -50,7 +50,7 @@
 <form method="post" action="<%RT->Config->Get('WebPath')%>/Approvals/index.html">
 
 <&| /Widgets/TitleBox, title => $title &>
-<& /Elements/ShowHistory , Object => $Ticket, ShowTitle => 0, ShowHeaders => 0, ShowDisplayModes => 0, ShowTitleBarCommands => 0, PathPrefix => RT->Config->Get('WebPath')."/Ticket/" &>
+<& /Elements/ShowHistory , Object => $Ticket, ShowTitle => 0, ShowHeaders => 0, ShowDisplayModes => 0, ShowActions => 0, PathPrefix => RT->Config->Get('WebPath')."/Ticket/" &>
 <hr />
 <& Elements/Approve, ticket => $Ticket, ShowApproving => 0 &>
 </&>
diff --git a/share/html/Approvals/Elements/Approve b/share/html/Approvals/Elements/Approve
index 860ea64..1560783 100644
--- a/share/html/Approvals/Elements/Approve
+++ b/share/html/Approvals/Elements/Approve
@@ -58,7 +58,7 @@
       <& /Ticket/Elements/ShowCustomFields, Ticket => $approving &>
 % }
 % if ($ShowHistory) {
-      <& /Elements/ShowHistory, Object => $approving, ShowTitle => 0, ShowHeaders => 0, ShowDisplayModes => 0, ShowTitleBarCommands => 0, PathPrefix => RT->Config->Get('WebPath')."/Ticket/" &>
+      <& /Elements/ShowHistory, Object => $approving, ShowTitle => 0, ShowHeaders => 0, ShowDisplayModes => 0, ShowActions => 0, PathPrefix => RT->Config->Get('WebPath')."/Ticket/" &>
 % }
     </div>
   </div>
diff --git a/share/html/Approvals/Elements/ShowDependency b/share/html/Approvals/Elements/ShowDependency
index 8b5a060..5f487e1 100644
--- a/share/html/Approvals/Elements/ShowDependency
+++ b/share/html/Approvals/Elements/ShowDependency
@@ -74,7 +74,7 @@ while (my $link = $approving->Next()) {
         $text .= $head;
     }
 
-    $text .= $m->scomp('/Elements/ShowHistory' , Object => $link->BaseObj, ShowTitle => 0, ShowHeaders => 0, ShowDisplayModes => 0, ShowTitleBarCommands => 0);
+    $text .= $m->scomp('/Elements/ShowHistory' , Object => $link->BaseObj, ShowTitle => 0, ShowHeaders => 0, ShowDisplayModes => 0, ShowActions => 0);
 
     $head .= $m->scomp('/Widgets/TitleBoxEnd');
     $text .= $m->scomp('/Widgets/TitleBoxEnd');
diff --git a/share/html/Articles/Article/History.html b/share/html/Articles/Article/History.html
index c1730e8..f562326 100644
--- a/share/html/Articles/Article/History.html
+++ b/share/html/Articles/Article/History.html
@@ -51,7 +51,7 @@
     Object => $article,
     ShowHeaders => 0,
     ShowDisplayModes => 0,
-    ShowTitleBarCommands => 0,
+    ShowActions => 0,
     DisplayPath => 'History.html',
     &>
 <%init>

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


More information about the rt-commit mailing list