Skip to content
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

"fixes duplicate xlink attribute on svg result" broke svgtex on Linux #15

Open
stroobandt opened this issue May 15, 2015 · 9 comments
Open

Comments

@stroobandt
Copy link
Contributor

This "fix" has caused a major regression and rendered svgtex completely useless on GNU/Linux (Xubuntu LTS 14.04). On all my browsers (Firefox, Chrome, etc.), I now only see one or more horizontal lines instead of any formula. Reverting that commit fixes the problem.

@tiarno
Copy link
Contributor

tiarno commented May 16, 2015

@serge-stroobandt , can you try the math expression xe for me (the two characters, no quotes)? That was my test case which continues to fail for me with Firefox and Chrome on Mac. Other equations render fine. I appreciate it if you can help me narrow the problem down.

@Klortho
Copy link
Contributor

Klortho commented May 18, 2015

For reference, this is the follow-on to this pull request, right? The commit in question has already been reverted in master, and this issue is about fixing the problem such that it doesn't break anything else?

FWIW, I know that when I worked on this recently, I found it necessary to pull in the "fix" in question.

@tiarno
Copy link
Contributor

tiarno commented May 18, 2015

Yes, that's the pull-request that was reverted. I worked on it this weekend and have had no success at all:

  • Without the fix I get 100% failure of the svg to display because of the duplicated hrefs.
  • With the fix I have not seen any failure.

So while I'm glad serge has success without the fix, I can't seem to do anything without it. I have the fix in my own fork so I can continue working with that.

@Klortho
Copy link
Contributor

Klortho commented May 18, 2015

Well, I have to say, I think reverting the fix in master was a bad idea. I remember that I only needed this fix when I upgraded to the latest phantomjs (2.0.0). @serge-stroobandt , is it possible that it's related to the phantomjs version number rather than the platform?

@tiarno
Copy link
Contributor

tiarno commented May 18, 2015

tested on Windows, same results--it must have the revereted fix in order to render the SVG. This is on Win7 now, with phantomjs 2.0.0. Will check phantomjs version later on the mac tonight. The error I continue to get is ::

 Attribute xlink:href redefined

Just a guess that earlier versions of webkit or phantomjs were more lenient.

@tiarno
Copy link
Contributor

tiarno commented May 19, 2015

okay, I installed phantomjs 1.9.8 and get success with the master branch of svgtex.

This isn't my project so I don't mean to presume, but what about creating a new branch for phantomjs_pre2.0 (without the fix) and one for the current phantomjs that includes the fix?

@Klortho
Copy link
Contributor

Klortho commented May 19, 2015

+1

@agrbin
Copy link
Owner

agrbin commented May 19, 2015

I don't like the solution with two branches.

Why wouldn't we inform the client (engine.js) which version we are running
by calling
http://phantomjs.org/api/phantom/property/version.html

at the beggining of the function listenLoop

svgtex/main.js

Line 302 in baacbe9

function listenLoop() {

and notifying this information to client with something like:
window.engine.setVersion

Then, the fix can be applied or not based on on the current phatnomjs
version. It seems like 3 added lines, what do you think?

On 19 May 2015 at 16:42, Chris Maloney [email protected] wrote:

+1


Reply to this email directly or view it on GitHub
#15 (comment).

@tiarno
Copy link
Contributor

tiarno commented May 19, 2015

I like it very much. Better than two branches for sure.

gwicke added a commit to wikimedia/mediawiki-services-mathoid that referenced this issue Jul 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants