Item13178: Don't permanently expand tags

The problem was the url matching to reverse from url to macro was based coded order of the list of macros. It needed to be "longest match" first. Also, any macro that is "empty" should not be substituted because it will not be reversable.
foswiki · Mar 1, 2015 · 87916c8 · 87916c8
1 parent 060fcbc
commit 87916c8
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 8 deletions.
diff --git a/TinyMCEPlugin/pub/System/TinyMCEPlugin/foswiki_tiny.uncompressed.js b/TinyMCEPlugin/pub/System/TinyMCEPlugin/foswiki_tiny.uncompressed.js
@@ -38,14 +38,15 @@ var FoswikiTiny = {
         return FoswikiTiny.foswikiVars[name];
     },
 
+//  This URL expansion is reversed by WysiwygPlugin::Handlers::postConvertURL()
     expandVariables: function(url) {
         for (var i in FoswikiTiny.foswikiVars) {
             // Don't expand macros that are not reversed during save
-            // Partial fix to Item13178
-            if ( i == 'WEB' ) continue;
-            if ( i == 'TOPIC' ) continue;
-            if ( i == 'SYSTEMWEB' ) continue;
+            // Part of fix to Item13178
+            if ( i == 'WEB' || i == 'TOPIC' || i == 'SYSTEMWEB' ) continue;
+            if ( FoswikiTiny.foswikiVars[i] == '' ) continue;   // Empty variables are not reversable
             url = url.replace('%' + i + '%', FoswikiTiny.foswikiVars[i], 'g');
+            //console.log( 'expandVariables ' + i + ' expanded to ' + FoswikiTiny.foswikiVars[i] );
         }
         return url;
     },
@@ -343,10 +344,13 @@ var FoswikiTiny = {
         var orig = url;
         var pubUrl = FoswikiTiny.getFoswikiVar("PUBURL");
         var vsu = FoswikiTiny.getFoswikiVar("VIEWSCRIPTURL");
+        var su = FoswikiTiny.getFoswikiVar("SCRIPTURL");
         url = FoswikiTiny.expandVariables(url);
         if (onSave) {
             if ((url.indexOf(pubUrl + '/') != 0) && 
-                    (url.indexOf(vsu + '/') == 0)) {
+                    (url.indexOf(vsu + '/') == 0) &&
+                    (su.indexOf(vsu) != 0) // Don't substitute if short URLs for view.
+                    ) {
                 url = url.substr(vsu.length + 1);
                 url = url.replace(/\/+/g, '.');
                 if (url.indexOf(FoswikiTiny.getFoswikiVar('WEB') + '.') == 0) {

diff --git a/WysiwygPlugin/lib/Foswiki/Plugins/WysiwygPlugin/Handlers.pm b/WysiwygPlugin/lib/Foswiki/Plugins/WysiwygPlugin/Handlers.pm
@@ -284,7 +284,10 @@ sub modifyHeaderHandler {
 # in URLs only
 use vars qw( @VARS );
 
-# The set of macros that get "special treatment" in URLs
+# The set of macros that get "special treatment" in URLs,  They have to end up
+# sorted based on their expanded length.  To convert from URL to MACRO it has to
+# be based upon longest match.  So _populateVars replaces this with the appropriately
+# sorted array.
 @VARS = (
     '%ATTACHURL%',
     '%ATTACHURLPATH%',
@@ -320,12 +323,38 @@ sub _populateVars {
         )
     );
 
+    # Item13178: The mapping between URL and vars needs to be longest match
+    # so the list must be sorted by length of the value.  Also, null entries
+    # should be omitted from the mapping, as they cannot be reversed.
+    my %varh;
+    my @exph = @exp;
+    foreach my $k (@VARS) {
+        my $val = shift @exph;
+        $varh{$k} = $val if ( defined $val );
+    }
+
+    my @nvars;
+    my @nexp;
+
+    # Do the sort by lenght.
+    foreach
+      my $k ( sort { length( $varh{$b} ) <=> length( $varh{$a} ) } keys %varh )
+    {
+        next unless $varh{$k};    # Omit empty variables, can't be reversed.
+        push @nvars, $k;
+        push @nexp,  $varh{$k};
+    }
+
+    @VARS = @nvars;    # Replace the vars list with the length sorted list.
+
+    # and build the list of values in order of @nvars.
     for my $i ( 0 .. $#VARS ) {
         my $nvar = $VARS[$i];
         $opts->{match}[$i] = $nvar;
-        $exp[$i] ||= '';
+        $nexp[$i] ||= '';    # Avoid undefined issues.
     }
-    $opts->{exp} = \@exp;
+    $opts->{exp} = \@nexp;
+
 }
 
 # callback passed to the TML2HTML convertor on each
@@ -334,6 +363,7 @@ sub expandVarsInURL {
     my ( $url, $opts ) = @_;
 
     return '' unless $url;
+    my $orig = $url;
 
     _populateVars($opts);
     for my $i ( 0 .. $#VARS ) {
@@ -343,6 +373,7 @@ sub expandVarsInURL {
 }
 
 # callback passed to the HTML2TML convertor
+# See also foswiki_tiny.js in TinyMCEPlugin,  which performs similar functions.
 sub postConvertURL {
     my ( $url, $opts ) = @_;
 

diff --git a/WysiwygPlugin/test/unit/WysiwygPlugin/TranslatorTests.pm b/WysiwygPlugin/test/unit/WysiwygPlugin/TranslatorTests.pm
@@ -1008,6 +1008,24 @@ HERE
 HERE
     },
 
+    {
+        # Test for short URL for view
+        # SMELL: This will fail with "shortest" URLs  (empty SCRIPTURLPATH).
+        exec => ROUNDTRIP | HTML2TML,
+        name => 'corruptedLinks_Item13178_a',
+        tml  => "   * [[%SCRIPTURL%/configure][configure]]
+   * [[%SCRIPTURLPATH%/configure][configure]]
+   * [[SomeTopic][some topic]]",
+        html => <<"HERE",
+<ul>
+<li> <a class='TMLlink' href="$Foswiki::cfg{DefaultUrlHost}$Foswiki::cfg{ScriptUrlPath}/configure">configure</a>
+<li> <a class='TMLlink' href="$Foswiki::cfg{ScriptUrlPath}/configure">configure</a>
+<li> <a class='TMLlink' href="$Foswiki::cfg{DefaultUrlHost}$Foswiki::cfg{ScriptUrlPaths}{view}/SomeTopic">some topic</a>
+</li>
+</ul>
+HERE
+    },
+
     # SMELL: No idea why we decode links,  but verify that it works anyway.
     #    {
     #        exec => ROUNDTRIP | TML2HTML | HTML2TML,