Doesn't show images in nodes

mjohnq3 - January 11, 2007 - 02:13
Project:Pdfview
Version:5.x-1.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed
Description

With Drupal 5 rc 1 images in nodes are not shown in pdf output. Only error is this rather obvious one: TCPDF error: Missing or incorrect image file: /drupal/files/images/Bottles (web).small.jpg.

The image file is in fact in that location. Everything else seems Ok.

#1

robin t - January 18, 2007 - 20:43

This looks like the report in http://drupal.org/node/76415 but for 5.0.

I have the same problem. Some added debug printout shows that it is _not_ the rewrite_image_url that's the problem. My custom debug URL rewrite just adds full path.
The complaint, however, is still the same:

TCPDF error: Can't open image file: /files/somefile.png

This filepath is the original (before the rewrite). In "theme_pdfview_node" I have added a print:
$body = pdfview_check_images($node->body);

print $body;

$pdf->SetFont(PDF_FONT_NAME_MAIN, '', PDF_FONT_SIZE_MAIN);
$pdf = theme('pdfview_html', $pdf, $node->body);

The img tag has changed OK, but TCPDF complains about the original src..?

This happens both for pages and for book pages.

Any hints?

#2

Egon Bianchet - January 18, 2007 - 20:59

Try to change

$pdf = theme('pdfview_html', $pdf, $node->body);

to

$pdf = theme('pdfview_html', $pdf, $body);

#3

robin t - January 19, 2007 - 06:58

Obvoiusly! I must have been tired and impatient last night. Of cause it works - doh!
This change should be comitted (if not already).
I will join the rewrite stuff test later (since I just hardcoded the path for now).

#4

Egon Bianchet - January 19, 2007 - 07:35
Status:active» fixed

Thanks, I've committed the change

#5

robin t - January 19, 2007 - 18:14
Status:fixed» needs work

Close but no cigar.
In the "_pdfview_rewrite_image_url" function the call to base_path() returns '/', so the str_replace removes all '/'es!
The base_path returns the path relative to the document_root (which is / at my installation). That line is no good.
What's needed is a function to return the full path of the installation and prepend that just before the return.
Something like the below works for me but I lack the necessary insight into drupal to make it general.

function _pdfview_rewrite_image_url($matches = array()) {
  $base_file_path = '/var/www/html/';
  $path = $matches[0];
  $path = str_replace($GLOBALS['base_url'].'/', '', $path);
  if (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC) == FILE_DOWNLOADS_PRIVATE) {
    $q = variable_get('clean_url', 0) ? '' : '?q=';
    $path = str_replace($q.'system/', '', $path);
  }
  $path = $base_file_path . $path;
  return $path;
}

Perhaps something like $base_url in the settings.php can be invented.

#6

Egon Bianchet - January 19, 2007 - 18:55

The $base_file_path role is played by K_PATH_IMAGES, inside the tcpdf configuration file.

Regarding the '/', I proposed this change to be tested http://drupal.org/node/76415#comment-164204 , but it looks like it's not working for everybody

#7

robin t - January 19, 2007 - 19:39

Your proposal in the other issue looks sane and will only eat the first / in my case.
(I saw this earlier but forgot about it again. Sorry for the dupe)

However, I cannot get K_PATH_IMAGES to make any difference no matter what I set it to. That's why I ventured into the above.
I always get the same path in the error message.

Anything else I could try?

BTW, issue 76415 is tagged to be about 4.x. This was opened on 5.x. Do you want to keep this separate or should I take this to the other issue?

#8

rubensans - January 27, 2007 - 09:36

Also happens to me I have test to change the image path some times and happens the same.

Best Regards.

#9

Egon Bianchet - January 27, 2007 - 09:56

I released the 5.x-1.0 version, it should fix the issues with base_path()

#10

mjohnq3 - January 29, 2007 - 00:20

Thanks. The latest version works fine so far.

#11

mwpeters - February 3, 2007 - 07:15

I am getting same messages and failures on nodes containing images.
Text only pages are not great, but no errors.

I have installed the module under /sites/all/modules/pdfview...
My pictures are coming from within the /files... branch.

Any idea what I should I be using for the settings in the "tcpdf_conf.php"?
The file example file contains an example using a DOS/WIN style windows drive, but I'm operating off a shared Linux host. I started at root and provided the path back. This may have nothing to do with my image issues, but I'm not at all confident of these settings or what effect they are having.

#12

Tobias Maier - February 3, 2007 - 08:19
Status:needs work» needs review

here comes my try to fix this

I hope it works
beside this it replaces tabs with whitespace (per coding standards)

AttachmentSize
pdfview.image_.patch 5.75 KB

#13

Egon Bianchet - February 3, 2007 - 11:40

Thanks for the effort Tobias, but I fail to understand what's the difference between your patch and the current approach ...

#14

Tobias Maier - February 3, 2007 - 12:06

first of all

<?php
$path
= str_replace($GLOBALS['base_url'].'/', '', $path);
if (
base_path() == '/' && $path[0] == '/') {
 
$path = substr($path, 1);
}
?>

// for clarification: $GLOBALS['base_url'] = '/'
line 1 replaces '//' with ''
this means if http://www.example.com/image.png gets handed over
"//" gets replaced (result: http:www.example.com/image.png)
line 2 looks if base_path = '/' and then it looks if the first sign is '/' but if the string is something like already a relative path or an absolute url it gets handed over to $path = str_replace(base_path(), '', $path); which replaces in this case every '/' with '' (result: http:www.example.comimage.png)

I don't think that the first line is necessary.
The other change solves the problem described above

<?php
if (base_path() == '/') {
  if (
$path[0] == '/') {
   
$path = substr($path, 1);
  }
}
?>

#15

Tobias Maier - February 3, 2007 - 12:17

here comes the patch again,
this time I removed every unnecessary change

btw. the

$path = str_replace($GLOBALS['base_url'].'/', '', $path);

is nearly the same line as
the existing one inside the if clause
$path = str_replace(base_path(), '', $path);

base_path() http://api.drupal.org/api/5/function/base_path

AttachmentSize
pdfview.image__0.patch 785 bytes

#16

mwpeters - February 4, 2007 - 07:22

Thanks Tobias,
Your patch fix and explanation worked for me.

Something like this deserves to get rolled into a release.

Mark

#17

Egon Bianchet - February 4, 2007 - 12:42

$GLOBALS['base_url'] is something like http://www.example.com or http://www.example.com/drupal ... I strip it from the path because image nodes use absolute urls instead of realtive, to keep working in rss feeds. So http://www.example.com/image.png should become image.png

Is there a real world case where $GLOBALS['base_url'] could become == '/' ?

The problem with slashed occurred because i was stripping the base path ($GLOBALS['base_path']) without checking if it was == '/'. But the current version does that check, so how is

<?php
 
if (base_path() == '/' && $path[0] == '/') {
   
$path = substr($path, 1);
  }
?>

different from

<?php
if (base_path() == '/') {
  if (
$path[0] == '/') {
   
$path = substr($path, 1);
  }
}
?>

?

#18

Tobias Maier - February 5, 2007 - 12:46

sorry, I misread $GLOBALS['base_url']
I read $GLOBALS['base_path'] so please forget #15
I fixed the patch

The problem with slashed occurred because i was stripping the base path ($GLOBALS['base_path']) without checking if it was == '/'.

But at the same time you check if the first sign is '/' and if this is NOT the case you fall back to the function which is known not to work with $GLOBALS['base_path']='/'

so you have to check if base_path() = / and if this is true check again, if it is necessary to strip the first slash.
but if you do both checks at once you fall back to the else clause and the old function runs again

<?php
if (base_path() == '/') {
  if (
$path[0] == '/') {
   
$path = substr($path, 1);
  }
}
else {
[...]
}
?>

I hope you understood what I mean.

AttachmentSize
pdfview.image__1.patch 1.21 KB

#19

Egon Bianchet - February 5, 2007 - 15:23
Status:needs review» fixed

OK, committed to HEAD, DRUPAL-5 and DRUPAL-4-7

Thanks!

#20

Anonymous - February 19, 2007 - 15:31
Status:fixed» closed
 
 

Drupal is a registered trademark of Dries Buytaert.