Generated PDF seemingly ignoring positions? #30

Closed
grofit opened this Issue Sep 5, 2012 · 11 comments

Comments

Projects
None yet
3 participants
@grofit

grofit commented Sep 5, 2012

I was just prototyping a small bit of functionality with the library and kept getting an issue where it positioned everything at the bottom left of the PDF when generating. Thinking it was something I had done I just outputted the default example "Hello World" but same thing, it positions it all down the bottom left overlapping everything.

I am using Chrome 21.0 and am using a manually viewed html file (file://...)

I would post up an example but its basically a html page with a button which triggers a javascript function to do EXACTLY the same as the hello world example.

Even the second page has the text down at the bottom left.

@grofit

This comment has been minimized.

Show comment Hide comment
@grofit

grofit Sep 5, 2012

Just to rule out the local file:// security issues (which I couldn't really see effecting anything internal to it creating pdfs) I hosted the file via IIS and same results. I tried in firefox but it wont display the outputted PDF it just displays the empty page with the data-uri.

grofit commented Sep 5, 2012

Just to rule out the local file:// security issues (which I couldn't really see effecting anything internal to it creating pdfs) I hosted the file via IIS and same results. I tried in firefox but it wont display the outputted PDF it just displays the empty page with the data-uri.

@grofit

This comment has been minimized.

Show comment Hide comment
@grofit

grofit Sep 5, 2012

Hmm interestingly I downloaded the jspdf file which is used by the demos and used that file and it works fine, so it seems to be an issue with the latest version on github.

grofit commented Sep 5, 2012

Hmm interestingly I downloaded the jspdf file which is used by the demos and used that file and it works fine, so it seems to be an issue with the latest version on github.

@dvdotsenko

This comment has been minimized.

Show comment Hide comment
@dvdotsenko

dvdotsenko Sep 5, 2012

Collaborator

@grofit

Cannot duplicate the issue.
21.0.1180.89 m Windows 7, 64b.

All tests run fine against master
All examples in example folder run fine (produce proper PDFs).

The examples are using uncompressed, actual .js files from the root of the repo. Tests use actual uncompressed .js files from the root of the repo. (Tests were ran on IE9, FF, Ch21 - all Windows)

I strongly suspect you messing up measurement units in pdf declaration or text positioning. Without actual failing example I would consider this issue NOT to exist.

Daniel.

Collaborator

dvdotsenko commented Sep 5, 2012

@grofit

Cannot duplicate the issue.
21.0.1180.89 m Windows 7, 64b.

All tests run fine against master
All examples in example folder run fine (produce proper PDFs).

The examples are using uncompressed, actual .js files from the root of the repo. Tests use actual uncompressed .js files from the root of the repo. (Tests were ran on IE9, FF, Ch21 - all Windows)

I strongly suspect you messing up measurement units in pdf declaration or text positioning. Without actual failing example I would consider this issue NOT to exist.

Daniel.

@dvdotsenko

This comment has been minimized.

Show comment Hide comment
@dvdotsenko

dvdotsenko Sep 5, 2012

Collaborator

Spoke too soon about IE9 passing tests. Refreshed the caches everywhere. Ch, FF still no issues. IE9 (IE8, 7 in emulation) produce new exciting failure conditions on text positioning. I love you, IE, in your special crooked ways! Will take a look at that.

Still, don't see Chrome failing.

Collaborator

dvdotsenko commented Sep 5, 2012

Spoke too soon about IE9 passing tests. Refreshed the caches everywhere. Ch, FF still no issues. IE9 (IE8, 7 in emulation) produce new exciting failure conditions on text positioning. I love you, IE, in your special crooked ways! Will take a look at that.

Still, don't see Chrome failing.

@grofit

This comment has been minimized.

Show comment Hide comment
@grofit

grofit Sep 6, 2012

I did make an example project with screenshots but as Github doesn't allow uploads I have made a JSFiddle:

http://jsfiddle.net/YSp7V/

For me that displays the "Hello World" text at the bottom left of the page, regardless of what values you give it.

grofit commented Sep 6, 2012

I did make an example project with screenshots but as Github doesn't allow uploads I have made a JSFiddle:

http://jsfiddle.net/YSp7V/

For me that displays the "Hello World" text at the bottom left of the page, regardless of what values you give it.

@MrRio

This comment has been minimized.

Show comment Hide comment
@MrRio

MrRio Sep 6, 2012

Owner

This does look like a bug. It should support the old argument order. If you
put the string first it works:

http://jsfiddle.net/YSp7V/2/

Owner

MrRio commented Sep 6, 2012

This does look like a bug. It should support the old argument order. If you
put the string first it works:

http://jsfiddle.net/YSp7V/2/

@grofit

This comment has been minimized.

Show comment Hide comment
@grofit

grofit Sep 6, 2012

Ah, didn't know that, I was using http://jspdf.com/ as the reference material.

Thanks for the quick feedback though!

grofit commented Sep 6, 2012

Ah, didn't know that, I was using http://jspdf.com/ as the reference material.

Thanks for the quick feedback though!

@dvdotsenko

This comment has been minimized.

Show comment Hide comment
@dvdotsenko

dvdotsenko Sep 6, 2012

Collaborator

@MrRio

Indeed a bug. The code that is responsible for swapping the args did not take into account that "primitive" types don't exist in JavaScript. All objects, including Number are objects and are transferred by reference. :(

http://jsfiddle.net/SPtUu/

I could have sworn that I tested both new and old API against the new builds. I was wrong.

Collaborator

dvdotsenko commented Sep 6, 2012

@MrRio

Indeed a bug. The code that is responsible for swapping the args did not take into account that "primitive" types don't exist in JavaScript. All objects, including Number are objects and are transferred by reference. :(

http://jsfiddle.net/SPtUu/

I could have sworn that I tested both new and old API against the new builds. I was wrong.

@dvdotsenko

This comment has been minimized.

Show comment Hide comment
@dvdotsenko

dvdotsenko Sep 6, 2012

Collaborator

Finally had dug up the real bug. It's not referential issue. It's minification optimization that clobbers the arg shuffle.

This:

if (typeof text === 'number') {
    text = arguments[2]
    x = arguments[0]
    y = arguments[1]
}

Is turned into this by minifier:

"number"===typeof i&&(c=a=i=c)

Apparently http://jsfiddle.net/SPtUu/ also minifies / compiles the snippet the same way, hence I saw the problem there. (http://jsfiddle.net/SPtUu/)

All unit tests used to run against UNminified version, which was NOT broken. I DID indeed have enough test coverage for both, new and old API and all passed on UNminified versions.

Now I has set up tests for both, mini and non-mini versions and will be testing both before committing.

Geesh, what a goose chase.

Collaborator

dvdotsenko commented Sep 6, 2012

Finally had dug up the real bug. It's not referential issue. It's minification optimization that clobbers the arg shuffle.

This:

if (typeof text === 'number') {
    text = arguments[2]
    x = arguments[0]
    y = arguments[1]
}

Is turned into this by minifier:

"number"===typeof i&&(c=a=i=c)

Apparently http://jsfiddle.net/SPtUu/ also minifies / compiles the snippet the same way, hence I saw the problem there. (http://jsfiddle.net/SPtUu/)

All unit tests used to run against UNminified version, which was NOT broken. I DID indeed have enough test coverage for both, new and old API and all passed on UNminified versions.

Now I has set up tests for both, mini and non-mini versions and will be testing both before committing.

Geesh, what a goose chase.

@dvdotsenko

This comment has been minimized.

Show comment Hide comment
@dvdotsenko

dvdotsenko Sep 6, 2012

Collaborator

@MrRio MrRio closed this in ba9495b Sep 6, 2012

@MrRio

This comment has been minimized.

Show comment Hide comment
@MrRio

MrRio Sep 6, 2012

Owner

Good work :)

That looked like a real pain to unravel.

Owner

MrRio commented Sep 6, 2012

Good work :)

That looked like a real pain to unravel.

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