Item14041: Broken calls to Foswiki::Func::getPubUrlPath make plugin non-functional

pencil
Priority: Normal
Current State: Closed
Released In: n/a
Target Release: n/a
Applies To: Extension
Component: DocumentViewerPlugin
Branches: master
Reported By: MichaelTempest
Waiting For:
Last Change By: MichaelDaum
The DocumentViewerPlugin contains calls like the following: Foswiki::Func::getPubUrlPath( $web, $topic, $attachment ); This results in empty iframes because the resulting path is not a valid path.

I fixed the problem on my installation as follows:
   my $path = Foswiki::Func::getPubUrlPath( web => $web, topic => $topic, attachment => $attachment );

   my $viewer =
      Foswiki::Func::getPubUrlPath( web => 'System', topic => 'DocumentViewerPlugin',
        attachment => 'ViewerJS/index.html' );

My server: This site is running Foswiki version v1.1.999_002, Release Foswiki-1.2.0_Beta_1, Plugin API version 2.3

DocumentViewerPlugin: Version 1.1, Release 27 Sep 2015

I also made (either of) the width and height parameters usable in combination with format, like this:
    if ( $format eq 'portrait' ) {
      if ($params->{height}) {
         $height = $params->{height};
         $width = int(724 * $height / 1024);
      }
      else {
         $width = $params->{width} || 724;
         $height = int(1024 * $width / 724);
      }
    }
    elsif ( $format eq 'landscape' ) {
      if ($params->{height}) {
         $height = $params->{height};
         $width = int(1024 * $height / 724);
      }
      else {
         $width = $params->{width} || 1024;
         $height = int(724 * $width / 1024);
      }
    }
    elsif ( $format eq 'screen' ) {
      if ($params->{height}) {
         $height = $params->{height};
         $width = int(1024 * $height / 768);
      }
      else {
         $width = $params->{width} || 1024;
         $height = int(768 * $width / 1024);
      }
    }
    else {
        $width  = $params->{width}  || 724;
        $height = $params->{height} || 1024;
    }

-- MichaelTempest - 06 Apr 2016

Foswiki::Func::getPubUrlPath( web => $web, topic => $topic, attachment => $attachment ); doesn't seem to be right either as the signature of this function is quite different....and different among Foswiki versions as well. Basically, the plugin flags itself incompatible to 1.1.x which is true because of the way the pub url is constructed. I fixed it differently using this patch:

diff --git a/lib/Foswiki/Plugins/DocumentViewerPlugin.pm b/lib/Foswiki/Plugins/DocumentViewerPlugin.pm
index db362ce..9e4bee9 100644
--- a/lib/Foswiki/Plugins/DocumentViewerPlugin.pm
+++ b/lib/Foswiki/Plugins/DocumentViewerPlugin.pm
@@ -16,13 +16,6 @@ our $NO_PREFS_IN_TOPIC = 1;
 sub initPlugin {
     my ( $topic, $web, $user, $installWeb ) = @_;
 
-    # check for Plugins.pm versions
-    if ( $Foswiki::Plugins::VERSION < 2.3 ) {
-        Foswiki::Func::writeWarning( 'Version mismatch between ',
-            __PACKAGE__, ' and Plugins.pm' );
-        return 0;
-    }
-
     Foswiki::Func::registerTagHandler( 'DOCUMENTVIEWER', \&_DOCUMENTVIEWER );
 
     return 1;
@@ -56,11 +49,8 @@ sub _DOCUMENTVIEWER {
         return
 '<noautolink><span class="foswikiAlert">DocumentViewerPlugin error: Attachment does not exist</span></noautolink>';
     }
-    my $path = Foswiki::Func::getPubUrlPath( $web, $topic, $attachment );
-
-    my $viewer =
-      Foswiki::Func::getPubUrlPath( 'System', 'DocumentViewerPlugin',
-        'ViewerJS/index.html' );
+    my $path = Foswiki::Func::getPubUrlPath(). "/$web/$topic/$attachment";
+    my $viewer = Foswiki::Func::getPubUrlPath() . '/%SYSTEMWEB%/DocumentViewerPlugin/ViewerJS/index.html';
 
     my $url = $viewer . '#' . $path;

This seems to be the most compatible way across Foswiki versions as far as I can see.

I like the other changes how format, height and weight are treated. But best would be to have separate bug items, one for the real bug, the other for the new feature. wink

-- MichaelDaum - 06 Apr 2016

I had wondered about Foswiki::Func::getPubUrlPath's signature. Your fix is better than mine smile

I'll raise another bug item for the new feature.

-- MichaelTempest - 06 Apr 2016

Item14044 raised for the new feature.

-- MichaelTempest - 07 Apr 2016

Philipp are you fine with the fix?

-- MichaelDaum - 11 Jul 2016

Unfortunately, I'm no longer able to maintain the plugin - but I'm happy with the changes you propose. May I put the plugin up for adoption? smile

-- PhilippGortan - 13 Jul 2016

Yes, please.

-- MichaelDaum - 14 Jul 2016
 
Topic revision: r8 - 02 Sep 2016, MichaelDaum
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy