New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug on pdf logo path #11691

Merged
merged 5 commits into from Dec 14, 2018

Conversation

Projects
None yet
6 participants
@SebBareyre
Copy link
Contributor

SebBareyre commented Dec 10, 2018

Questions Answers
Branch? develop
Description? Fix bug on pdf logo path in invoice for example. Change absolute to relative path on img src attribute
Type? bug fix
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket? -
How to test? Generate an invoice. The logo must be displayed without overlapping the text.

This change is Reviewable

[CO] Fix bug on pdf logo path
Fix bug on pdf logo path in invoice for example. Change absolute to relative path on img src attribute

@ntiepresta ntiepresta self-assigned this Dec 11, 2018

@ntiepresta

This comment has been minimized.

Copy link

ntiepresta commented Dec 11, 2018

Hello,
After testing this PR, the warning alert message is displayed(when updating status order 'payment accepted') and the logo isn't correctly displayed on invoice.
logo modif
logo modif4
It Should displayed correctly the logo on invoice.
logo modif 5

@SebBareyre

This comment has been minimized.

Copy link
Contributor

SebBareyre commented Dec 11, 2018

@ntiepresta I think your invoice logo is too big in size... In my case, without my PR, the logo doesn't appear...

@marionf

This comment has been minimized.

Copy link
Contributor

marionf commented Dec 12, 2018

@SebBareyre

This is the image used

tshirt jaune

Without your PR, the image is well resized on the invoice:

capture d ecran_751

With your PR, the text overlays the image

capture d ecran_752

@SebBareyre

This comment has been minimized.

Copy link
Contributor

SebBareyre commented Dec 12, 2018

I can't see how, the image path (relative instead of absolute) could influence the image size... (cc @mickaelandrieu )

@Quetzacoalt91

This comment has been minimized.

Copy link
Member

Quetzacoalt91 commented Dec 12, 2018

Well, you rely on a uri, which is slightly different from a relative path in some case.

Can you please provide more details on the bug you are fixing here?

@SebBareyre

This comment has been minimized.

Copy link
Contributor

SebBareyre commented Dec 13, 2018

@Quetzacoalt91 : In my case, with an absolute path, the image doesn't appear and with the relative path, the image appear well...

@Quetzacoalt91

This comment has been minimized.

Copy link
Member

Quetzacoalt91 commented Dec 13, 2018

There is probably something else to solve, eventually something related to a special character like a space, or a windows path which is not understood properly by the PDF generator.

Could you provide the values of these 2 constant when the code is run?

@SebBareyre

This comment has been minimized.

Copy link
Contributor

SebBareyre commented Dec 13, 2018

@Quetzacoalt91 :
absolute path : /var/www/clients/client12/web203/web/releases/20181207122157/img/le-laboratoire-de-biarritz-1501169561.jpg
relative path : /img/le-laboratoire-de-biarritz-1501169561.jpg

@SebBareyre

This comment has been minimized.

Copy link
Contributor

SebBareyre commented Dec 13, 2018

@Quetzacoalt91 : ok I understood, with a relative path, it can't calculate the logo width and height..

@SebBareyre
Copy link
Contributor

SebBareyre left a comment

fix calculation of logo width and height

@Quetzacoalt91
Copy link
Member

Quetzacoalt91 left a comment

I actually reproduce the original issue, my invoices don't have any logo without this PR. Thank you @SebBareyre, let's work together to make it displayed properly.

Show resolved Hide resolved classes/pdf/HTMLTemplate.php Outdated
Show resolved Hide resolved classes/pdf/HTMLTemplate.php Outdated

@Quetzacoalt91 Quetzacoalt91 changed the title [CO] Fix bug on pdf logo path Fix bug on pdf logo path Dec 13, 2018

@Quetzacoalt91 Quetzacoalt91 added this to the 1.7.6.0 milestone Dec 13, 2018

Quetzacoalt91 and others added some commits Dec 13, 2018

Update classes/pdf/HTMLTemplate.php
Co-Authored-By: SebBareyre <sebastien@ohmyweb.fr>

@ntiepresta ntiepresta removed their assignment Dec 13, 2018

@ntiepresta ntiepresta added QA ✔️ and removed waiting for QA labels Dec 13, 2018

@Quetzacoalt91 Quetzacoalt91 merged commit 6fc6d2a into PrestaShop:develop Dec 14, 2018

1 of 2 checks passed

Codacy/PR Quality Review Not up to standards. This pull request quality could be better.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Quetzacoalt91

This comment has been minimized.

Copy link
Member

Quetzacoalt91 commented Dec 14, 2018

Thank you @SebBareyre

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment