Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upBad Image URL in the Social Cards when data-image Attribute is None #128
Comments
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Thanks for the heads up. I'll investigate. |
shawnmjones
self-assigned this
Sep 6, 2018
shawnmjones
added
the
bug
label
Sep 6, 2018
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ibnesayeed
Sep 6, 2018
Member
MementoEmbed/mementoembed/services/memento.py
Line 106 in a4a9df9
On a quick note I think replacing above line with the following should do the trick:
output['best-image-uri'] = best_image_uri or ''
MementoEmbed/mementoembed/services/memento.py Line 106 in a4a9df9 On a quick note I think replacing above line with the following should do the trick: output['best-image-uri'] = best_image_uri or '' |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
shawnmjones
Sep 6, 2018
Collaborator
Thanks @ibnesayeed, but the JavaScript should be less brittle. Only fixing the problem in /services/memento.py
ignores the machine endpoints in /services/product.py
.
Thanks @ibnesayeed, but the JavaScript should be less brittle. Only fixing the problem in |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ibnesayeed
Sep 6, 2018
Member
Well, it has to do with how Python converts None
to string. While None
is a falsy object in boolean context, when converted to string it does not return a falsy string (i.e. an empty sting).
>>> str(None)
'None'
This means we need to be careful where we not want that to happen. One way to fix this issue for various different places where it might be used is to change the default value of max_score_image
from None
to an empty string at the following line.
MementoEmbed/mementoembed/imageselection.py
Line 113 in 7d628bb
Well, it has to do with how Python converts >>> str(None)
'None' This means we need to be careful where we not want that to happen. One way to fix this issue for various different places where it might be used is to change the default value of MementoEmbed/mementoembed/imageselection.py Line 113 in 7d628bb |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
shawnmjones
Sep 6, 2018
Collaborator
The problem is actually larger that that. This is part of it, but there is a larger issue with the image selection code. I'll give details soon.
The problem is actually larger that that. This is part of it, but there is a larger issue with the image selection code. I'll give details soon. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ibnesayeed
Sep 6, 2018
Member
A similar issue often surfaces in JS as well where falsy null
or undefined
will be converted to strings as a word, not an empty string. I usually use a similar logical operator trick to canonicalize such variables in string context as it can be seen in the Reconstructive repo.
rels.first && rels.first.href || ''
The above JS line can be interpreted as:
(rels.first && rels.first.href) || ''
The way logical operators are executed is if there is an AND between two statements, the value of the first one will be returned if it is falsy, otherwise the value of the second will be returned instead. And if there is an OR then the first one will be returned if it is truthy, otherwise the value of the second one will be returned instead.
So, in the above example it means check the value of rels.first.href
if and only if rels.first
is not null
(to avoid an exception like cannot read property 'href' of null object
) and return the value of rels.first.href
if it is not falsy. Otherwise the statement in parenthesis returns falsy (due to the first one or the second) then return the value after the OR operator, which is an empty string.
A similar issue often surfaces in JS as well where falsy rels.first && rels.first.href || '' The above JS line can be interpreted as: (rels.first && rels.first.href) || '' The way logical operators are executed is if there is an AND between two statements, the value of the first one will be returned if it is falsy, otherwise the value of the second will be returned instead. And if there is an OR then the first one will be returned if it is truthy, otherwise the value of the second one will be returned instead. So, in the above example it means check the value of |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
shawnmjones
Sep 6, 2018
Collaborator
The MementoEmbed code will be modified to provide a default image if none is found.
The MementoEmbed code will be modified to provide a default image if none is found. |
hariharshankar commentedSep 6, 2018
For some Mementos, the
/services/product/socialcard/
API returnsdata-image="None"
. In these scenarios, thegenerate_cards()
JS method creates the social card image in#me-image
withsrc="None"
.Either the check in the
generate_cards()
could account forNone
or the<blockquote>
tag could omit properties that areNone
.