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

Removed use of SvgPathSeg, fixing Chrome error #1552

Closed
wants to merge 3 commits into from
Closed

Removed use of SvgPathSeg, fixing Chrome error #1552

wants to merge 3 commits into from

Conversation

bottomline
Copy link

No description provided.

@bottomline bottomline changed the title Bugfix/path click fix Fix Chrome issue with pathseglist Jan 22, 2016
@bottomline bottomline changed the title Fix Chrome issue with pathseglist Removed use of SvgPathSeg, fixing Chrome error Jan 22, 2016
@perlun
Copy link

perlun commented Feb 3, 2016

@masayuki0812 - any chance we can get this merged (and a new version released) in the near future? The current version is broken in Chrome 48, which was rolled out some week ago, so it's quite critical to get this fixed.

(For now, using the version from bottomline, but it would be nice to be able to get this into an "official" 0.4.11 release or similar.)

Thanks in advance, and huge thanks for a great FOSS project! Do let us know if this is a time problem; I'm sure there are volunteers that can help with maintaining the project if needed.

@ericpias
Copy link

ericpias commented Feb 3, 2016

The code change in this pull request uses a reserved javascript keyword (char) as a variable name in several spots. In testing it with yuicompressor, it fails. It is a simple matter to change it. When I get more comfortable with github I will change it if not already fixed. I would hate for it to get into a released version that way. Thanks for for fixing the issue!

@@ -1,3 +1,312 @@
// PathData object is based upon `Source`, here https://github.com/jarek-foksa/path-data-polyfill.js/blob/master/path-data-polyfill.js

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nature of the changes in this file strike me as a polyfill. I'm not an active contributor to this project, but as an interested consumer, I would think it might make more sense if this were moved to src/polyfill.js.

For what it's worth, I favour this PR as a whole over #1564, as we should be providing backbridges to legacy APIs, not forwardbridges to current ones.

@ggamir
Copy link
Contributor

ggamir commented Feb 16, 2016

Please... everyone's browsers are autoupdating to the latest version... it's only a matter of time... 😢

@perlun
Copy link

perlun commented Feb 17, 2016

I agree, it's becoming a real issue. Is the project abandoned?

@aendra-rininsland
Copy link
Member

I've merged #1564 into dev so am closing this as a duplicate.

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

Successfully merging this pull request may close these issues.

6 participants