Open Zegnat opened 6 years ago
Seems sensible to me, especially with the overwriting aspect. I can't think of a case where the ability to intentionally set one of these fields to an empty string would be important.
text
should also specify the following
text: " "
considered valid?<style>
and <script>
elements be dropped before?<img>
elements be replaced with their src
or alt
?related: #20
While #15 is still waiting on resolving, I do think the text
property should follow #17 and include the style
, script
, and img
language.
How about this?
- add the following keys to the hash of the key with name url, unless the key is already set:
hreflang
: the value of the element’shreflang
attribute, if defined and not an empty string, after removing all leading/trailing whitespacemedia
: the value of the element’smedia
attribute, if defined and not an empty string, after removing all leading/trailing whitespacetitle
: the value of the element’stitle
attribute, if defined and not an empty string, after removing all leading/trailing whitespacetype
: the value of the element’stype
attribute, if defined and not an empty string, after removing all leading/trailing whitespacetext
: thetextContent
of the element, if not an empty string, after:- dropping any nested
<script>
&<style>
elements;- replacing any nested
<img>
elements with theiralt
attribute, if present; otherwise theirsrc
attribute, if present, adding a space at the beginning and end, resolving the URL if it’s relative- removing all leading/trailing whitespace.
I'm +1 on most of this, but not sure about the "and not an empty string" part. I'm not sure it would cause any problems, but it feels a bit weird to me -- like the parser is trying to fix a publisher mistake. I'll defer to others on that.
I’m not sure it would cause any problems, but it feels a bit weird to me — like the parser is trying to fix a publisher mistake.
It was my understanding that the entire rel-urls
object existed only to assist consumers, it was drafted specifically for @kevinmarks’ universal feed parser according to the brainstorm. That might have been why I was overzealous “to fix a publisher mistake”, I thought the whole point was to collect the most accurate information available for the specified URL.
In fact, accepting these attributes with an empty string value (""
) isn’t even valid HTML. hreflang
“must be a valid BCP 47 language tag” and type
“must be a valid MIME type string”. Neither of those accept empty strings. I am not saying microformats parsers should validate those values, but discarding invalid empty values seemed like a low hanging fruit. Apparently I was wrong.
I will concede to not having encountered it in the wild, with the footnote that I have never used the rel-urls
object for anything. Now thinking about running an in browser experiment to see if I can find examples where skipping empty string values will net different results.
That said: point two of this issue still stands.
We need to resolve that “if any” means for text
. Do we skip empty values there? Before or after stripping white space? I still propose we change “if any” to “if not an empty string” unless someone interprets it differently.
Apart from that, do we want to strip leading/trailing whitespace from the attributes, or do we want to keep all whitespace as authored?
New proposal, no skipping of empty values, but does strip surrounding whitespace:
- add the following keys to the hash of the key with name url, unless the key is already set:
hreflang
: the value of the element’shreflang
attribute, if defined, after removing all leading/trailing whitespacemedia
: the value of the element’smedia
attribute, if defined, after removing all leading/trailing whitespacetitle
: the value of the element’stitle
attribute, if defined, after removing all leading/trailing whitespacetype
: the value of the element’stype
attribute, if defined, after removing all leading/trailing whitespacetext
: thetextContent
of the element, if not an empty string, after:- dropping any nested
<script>
&<style>
elements;- replacing any nested
<img>
elements with theiralt
attribute, if present; otherwise theirsrc
attribute, if present, adding a space at the beginning and end, resolving the URL if it’s relative- removing all leading/trailing whitespace.
Current spec text:
I think this should clarify 1) what to do with empty attribute values, and 2) what exactly “if any” means.
Currently parsers check the existence of an attribute, but never check their value. Thus empty attributes (e.g.
hreflang=""
) will lead to empty strings being added to the object inrel-urls
. This means the objects inrel-urls
will always match with the authored HTML.The drawback of this is that a link parsed later in the source document may not overwrite the empty value any more because the key is already present, even if they add information. Example:
Will lead to (edited to only show the affected property):
Clearly the empty string adds no information about the URL
#a
, but because it came first in source order that’s the one we keep.Currently the
text
property should only be added if there is “any” text content. But nowhere does it define what text content means or when we consider there to be none.This has already lead to a difference in implementations. The Python parser will add an empty
text
property, the PHP parser will not. Example:In PHP:
In Python:
I believe that, in both cases, storing an empty string value for any property is useless and adds no data. With the overwriting logic part of the parser, we may even miss out on information. Therefore I would propose rewriting the spec as follows: