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

Option to warn about use of flow text elements #49

Closed
oberstet opened this Issue Mar 13, 2016 · 14 comments

Comments

Projects
None yet
5 participants
@oberstet
Collaborator

oberstet commented Mar 13, 2016

When using flow text - as opposed to standard text - in Inkscape, it will save the SVG with flowRoot elements, which pretty much is only understood by Inkscape itself. All browsers will fail.

It would be very nice/convenient if scour could resolve, or at least warn of this problem.

@Perhelion

This comment has been minimized.

Perhelion commented Mar 13, 2016

+1 absolutely !!

@Ede123

This comment has been minimized.

Member

Ede123 commented Mar 14, 2016

I vote against implementing a workaround for this in Scour:

  • First of all it won't be easy and would require a lot of complex (and error prone) code. Basically we'd have to add full support for text rendering with precise text metrics and proper font support to be able to convert flowRoots to normal text objects. In my opinion this is far beyond the scope of Scour.
  • Inkscape already implements functionality to convert flowRoots to normal text. No need to duplicate functionality (especially since I assume font support in Inkscape is better than everything we'd be able to offer in Scour with reasonable effort).
  • As soon as the syntax for flowed text in the SVG 2 specification is ready Inkscape will support it and probably there will be an automated way to convert flowRoots to this syntax. At that point the feature will become unnecessary.

If anybody wants to put any effort into this it would probably be a much better idea to solve this issue at its root and make Inkscape add an SVG 1.1 compliant fallback automatically when using flowRoot. I think the latest Inkscape bug for this is at [1] and it already had a partial fix that could serve as a start.

If we want to add any functionality to Scour at all I'd add an option to completely remove all flowRoots and document it accordingly (e.g. "remove flowed text elements incompatible with most renderers"). Missing text will serve as a clear notice to the user that non-standard functionality had been used (and removed) and it would be an easy possibility to produce standard conforming SVGs.

Not at last it would also be enough to solve probably 90 % of issues encountered at Wikimedia projects (which I guess is a major motive for @Perhelion), because the infamous "black rectangle issue" (which is how MediaWiki's SVG renderer libRSVG renders flowRoots) is mostly caused by empty flowRoots whose existence people are not even aware of...

@Eitot

This comment has been minimized.

Contributor

Eitot commented Mar 20, 2016

I think this is normal behaviour and should not be addressed per Ede123’s scope argument. All editors have non-standard output that Scour cannot deal with, because it presupposes standardised SVG. When compatibility with an editor is important, the option --keep-editor-data should be used.

@Perhelion

This comment has been minimized.

Perhelion commented Mar 21, 2016

Yes, your are right, it is more like a "problem" of Inskcape and I can also more agree @Eitot. But as we can see Inkscape has since 10 years not solved this feature request ("bug"): https://bugs.launchpad.net/inkscape/+bug/167335 (where was suggested to move this to Inkscape namespace)
In fact it is Inkscape proprietary code which no other editor (or viewer) can render. So I don't understand the whole usage of this feature at all.

@Eitot

This comment has been minimized.

Contributor

Eitot commented Mar 21, 2016

It is not just Inkscape, but pretty much all editors do this. SVG is not exclusively a web format and its extensibility through namespaces is one of its features. That way programs can use it for their own purposes. There are lots of reasons why one would want to use Inkscape. There is no reason to limit it to what a browser is capable of rendering.

@oberstet

This comment has been minimized.

Collaborator

oberstet commented Mar 22, 2016

Leaving all good arguments made above aside (such as "standard" and "complexity" and ..), as a user, I just want to have a work flow from Inkscape down to browsers - and browsers don't understand flowRoot.

The bug https://bugs.launchpad.net/inkscape/+bug/167335 linked by @Perhelion pretty much nails it.

Inkscape should provide an option to save without flowed text. Due to the complexity, it should be done in Inkscape, not Scour. Hell, actually, what Scour does should be built into Inkscape: save for Web.

Anyway. Right now, the only way is to simply ask designers not to use flowed text. Automatic cleaning would be better than asking, but at least, Scour could warn when flowed text is encountered. So I am changing the title of the issue ..

@oberstet oberstet changed the title from Easy way to resolve flowRoot to Warn when flowed text is used Mar 22, 2016

@oberstet

This comment has been minimized.

Collaborator

oberstet commented Mar 22, 2016

@Ede123 After rehinking, I guess the behavior your describe where scour would have an option to aggressively remove (while noticing) any floatRoot elements is even better! That would be quite useful. The designer is warned, and the downstream web developer is happy too.

@oberstet oberstet changed the title from Warn when flowed text is used to Option to remove flow text elements Mar 22, 2016

@Eitot

This comment has been minimized.

Contributor

Eitot commented Mar 22, 2016

I do not like that approach that much, because it is too specific to this one case. The underlying problem of float root is that it is (1) editor-specific markup at this point that is (2) anticipated to be standardised in the next release of SVG. I think a more future-proof approach is to group current and draft elements by SVG release and offer an option to remove draft elements instead.

As for the warning, I think a better documentation is the way to go.

@codedread

This comment has been minimized.

Collaborator

codedread commented Mar 22, 2016

Wow 10+ year old bug in Inkscape :)

Maybe time to focus on tools that output compliant SVG that browser understand. Switch to using https://github.com/SVG-Edit/svgedit ? :D

@oberstet oberstet changed the title from Option to remove flow text elements to Option to warn about use of flow text elements Apr 1, 2016

@oberstet

This comment has been minimized.

Collaborator

oberstet commented Apr 1, 2016

My view: I don't know about the evolution details of the SVG standard, and who is "correct" (Inkscape rightly implementing an 1.2 feature, and browser screwing up - or the feature being implemented in Inkscape never being fully spec'ed / released officially). And honestly, I don't care.

What I do care about is the fact that an SVG created in Inkscape using flow text will not work in any browser. This breaks workflows for us.

In other words: IMO, scour should have an option at least to warn about such use. I agree, transforming flow text into regular text is too complex. I am fine with warning alone, as the designer needs to fix it.

@Ede123

This comment has been minimized.

Member

Ede123 commented Apr 2, 2016

Thinking more about this I agree with @Eitot that a fix specifically addressing flowRoots is probably not a good way.

I therefore suggest a more general (and probably much more useful) approach:
What if why implemented an option to check the generated SVG for W3C validity? Since flowRoot was only part of a draft specification it will be considered invalid (which is wat we want) but at the same time we'd be able to catch many other problems.

Only question would be how to implement this feature:

  • I checked if a Python module for local validation of SVG files was available but couldn't find something useful so far (anybody else knows about something?).
    The best I could come up with so far is XML validation with lxml, but we'd have to figure out how to tell lxml what a valid SVG file looks like ourselves.
  • Probably more straightforward (at the cost of an HTML request) would probably be to use the W3C Markup Validation Service API. This way we only have to implement the infrastructure and don't have to care about the actual validation which will always be up to date thanks to W3C.

What do you guys think? If you like the idea I might try to have a shot at an implementation.

@oberstet

This comment has been minimized.

Collaborator

oberstet commented Apr 2, 2016

option to check the generated SVG for W3C validity

I don't care about theoretical "W3C validity". I care about real-world browser compatibility.

IMO "validating SVG" is way over the top, and way too complex/ambigious. I prefer a simple/pragmatic check: is there any flowText element .. and if so, warn. Problem solved. (The problem being designer accidently introducing flowText). Calling out to a web service is also a no go for a local checker/sanitizer.

@Ede123

This comment has been minimized.

Member

Ede123 commented Apr 2, 2016

Sorry to disagree but W3C validity ensures browser compatibility in most cases... There's nothing theoretical about it. Validation for W3C standards is everything but ambiguous and as long as we don't try to implement it ourselves (that would also be a no-go for me) I don't see how it should be complex.

Checking for one arbitrary non-standard SVG element might solve one very specific issue (that is only a symptom of a very specific implementation detail of Inkscape). If we add an exception for that we can continue to add many more specific warnings with the same argumentation to a point where it would have been easier to just implement proper validation.

I can accept your concerns regarding HTTP requests, but if a local solution can be found I'd clearly favor it.

@oberstet

This comment has been minimized.

Collaborator

oberstet commented Apr 2, 2016

The original issue is addressed by #53

If you want to go further, like checking for W3C validity (whatever that means), cool. But this is likely to be a bigger thing, I'd like to merge above in the meantime.

@oberstet oberstet closed this in #53 Apr 18, 2016

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