astefanutti / decktape

PDF exporter for HTML presentations
MIT License
2.21k stars 177 forks source link

viewBox not used for inline SVG ? #5

Closed hadim closed 9 years ago

hadim commented 9 years ago

I am maybe wrong but the rendered SVG (<svg width="100%" data-src="schema.svg"></svg>) with decktape do not use the viewBox attribute. See the screenshot below.

While it renders correctly with both last version of Chromium and Firefox.

svg

hadim commented 9 years ago

Not that I also use svg {height: 100%;} in my css.

hadim commented 9 years ago

In fact viewBox is used but it seems that width and height attributes of the SVG file are used in priority over the viewBox because when I remove the SVG attributes width and height then the SVG is correctly rendered as in browser.

Any idea to make it a permanent fix ? Should I open an issue for phantomjs ?

astefanutti commented 9 years ago

A suggested in #4, that may be better to directly report this point in Qt issue tracker directly:

https://bugreports.qt.io/browse/QTBUG

hadim commented 9 years ago

Removing width and height attributes does not work well. Qt Webkit needs to be fixed. I filled an issue there : https://bugreports.qt.io/browse/QTBUG-46986

hadim commented 9 years ago

@astefanutti I'd like to test it with Qt 5.5. Is the best way replace phantomjs/src/qt source code with Qt 5.5 ? Nothing more to do ?

astefanutti commented 9 years ago

Sounds like a good idea. You can replace the phantomjs/src/qt. An other approach would be to apply the diff between 5.4.1 and 5.5.0.

There may be some PhantomJS build resources to update like: https://github.com/ariya/phantomjs/commit/68ca7c3b5d98b1d3df6db2dffee03676c7920128#diff-d6d9dd3e06474412f0bb613c0b88248bR2

Let me know how it goes.

hadim commented 9 years ago

Well since Qt example browser give the same result as with phantomjs/decktape I won't bother switch phantomjs to 5.5. Instead I will build qt 5.5 from source with browser example and do the test from the browser example.

astefanutti commented 9 years ago

Great idea, that'd be much easier.

hadim commented 9 years ago

I confirm the bug is still present in 5.5.1, tested with examples/webkitwidgets/browser.

astefanutti commented 9 years ago

OK, thanks for head up.

hadim commented 9 years ago

The issue does not appear using QtWebEngine (tested with examples/webengine/quicknanobrowser/quicknanobrowser). And after reading some posts about QtWebKit/QtWebEngine, I saw that QtWebKit is not supported anymore... So I guess the best is to wait PhantomJS to move to QtWebEngine as they said there : https://github.com/ariya/phantomjs/issues/12931

hadim commented 9 years ago

I close since it depends on QtWebkit.