[Rt-commit] rt branch, 4.2/showhistory-paths-and-titlebars, created. rt-4.2.4-40-g1aa631b
Kevin Falcone
falcone at bestpractical.com
Tue May 27 17:20:13 EDT 2014
The branch, 4.2/showhistory-paths-and-titlebars has been created
at 1aa631b9bd7f23deb1ca4aef767dd41151cf9a2a (commit)
- Log -----------------------------------------------------------------
commit 324e22d3c8118730201a50f8dcb15b2a3131e7f8
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.
Since /Elements/ShowHistory isn't directly callable and the JS helper doesn't
pass extra arguments, there are currently no ways to pass a malicious
PathPrefix in from user supplied data.
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/Approvals/Elements/ShowDependency b/share/html/Approvals/Elements/ShowDependency
index 8b5a060..20de04e 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, ShowTitleBarCommands => 0, PathPrefix => RT->Config->Get('WebPath')."/Ticket/");
$head .= $m->scomp('/Widgets/TitleBoxEnd');
$text .= $m->scomp('/Widgets/TitleBoxEnd');
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 18957ceb9977fe3f0bbc33d86850fe09f718ffe9
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 18526eba7eecf37ce6ba1b2309df0f95a4afaf2a
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 1aa631b9bd7f23deb1ca4aef767dd41151cf9a2a
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 20de04e..ae33769 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, PathPrefix => RT->Config->Get('WebPath')."/Ticket/");
+ $text .= $m->scomp('/Elements/ShowHistory' , Object => $link->BaseObj, ShowTitle => 0, ShowHeaders => 0, ShowDisplayModes => 0, ShowActions => 0, PathPrefix => RT->Config->Get('WebPath')."/Ticket/");
$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