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

Display of contributors table broken in Overcast client because of srcset attribute #778

Closed
jancbeck opened this issue Sep 18, 2015 · 3 comments

Comments

@jancbeck
Copy link

This is how the contributors table looks like in Overcast in an iPhone 6+:

overcast
(Instacast, for comparision)

Clearly Overcast doesn't shine here, but I according to FeedValidator it's kinda right:

line 63, column 0: content:encoded should not contain srcset attribute (340 occurrences) [help]
Some feed elements are allowed to contain HTML. However, some HTML attributes, like onclick, are potentially dangerous and could cause unwanted side effects in browser-based news aggregators. In a perfect world, these dangerous attributes would be stripped out on the client side, but it's not a perfect world, so you should make sure to strip them out yourself.

The list of dangerous attributes varies from browser to browser, and even from browser version to browser version. As such the Feed Validator takes a white-list approach, and only accepts the following attributes:
abbr, accept, accept-charset, accesskey, action, align, alt, axis, border, cellpadding, cellspacing, char, charoff, charset, checked, cite, class, clear, cols, colspan, color, compact, coords, datetime, dir, disabled, enctype, for, frame, headers, height, href, hreflang, hspace, id, ismap, label, lang, longdesc, maxlength, media, method, multiple, name, nohref, noshade, nowrap, prompt, readonly, rel, rev, rows, rowspan, rules, scope, selected, shape, size, span, src, start, summary, tabindex, target, title, type, usemap, valign, value, vspace, and width

I'm not sure if this is really an issue with Podlove Publisher since Instacast has no problems displaying images with srcset and actually even correctly shows a retina resolution image on an iPhone 6. I also don't see any security implications using srcset in content:encoded since src is allowed. FeedValidator is probably just outdated here and in fact there is already a pull request.

So this is issue is probably a wontfix but should be there for others to see and discuss it. The offending lines are lib/model/image.php#L218-L219
To fix this issue one could either test for is_feed or modify the setAttribute method in \Podlove\DomDocumentFragment to only accept whitelisted attributes.

@eteubert
Copy link
Member

As you're saying, I don't think this should be fixed in the Publisher.

FeedValidator is nothing but an endless source of annoyances (see also: why they won't allow https as valid URL scheme).
However, I'm confident that reporting this to Overcast will eventually result in a fix.

@eteubert
Copy link
Member

I just reached out to Marco, the Overcast dev.

@eteubert
Copy link
Member

Doesn't seem fixed yet but nothing much we can do except wait.

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

No branches or pull requests

2 participants