[
https://issues.apache.org/jira/browse/WICKET-6042?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15036681#comment-15036681
]
ASF GitHub Bot commented on WICKET-6042:
----------------------------------------
Github user sebfz1 commented on the pull request:
https://github.com/apache/wicket/pull/143#issuecomment-161439689
Hi Tobias,
> Thank you for the time you spend to review the code!
You are welcome, thanks a lot for your time too! :)
> Any other things to be changed?
Well... yes ! :) You have removed the null check on the *model* in
`#buildSrcAttribute`, but there is still no null check on the model *object*.
Sorry if I had not been clear enough.
As I wrote previously, I see 2 options to handle this:
1/ Check null *object*, and do not render the src tag. This will lead the
image to not show at all (well, at least FF works like that)
```java
protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
{
if (srcModel.getObject() != null)
{
tag.put("src", srcModel.getObject().toString());
}
}
```
2/ use `String#valueOf`. This will lead to the src tag to have null value
and display a broken image, which can also make sense to alert without throwing
a runtime NPE...
```java
protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
{
tag.put("src", String.valueOf(srcModel.getObject()));
}
```
I have a slight preference for the 2nd choice. I guess you will opt for the
first choice; so if everyone is fine with this first choice, that's fine for me
too! (@svenmeier , @martin-g)
Thanks again,
Sebastien
> Implementation of ExternalImage component
> -----------------------------------------
>
> Key: WICKET-6042
> URL: https://issues.apache.org/jira/browse/WICKET-6042
> Project: Wicket
> Issue Type: Improvement
> Components: wicket
> Affects Versions: 7.1.0, 8.0.0-M1
> Reporter: Tobias Soloschenko
> Assignee: Tobias Soloschenko
> Labels: features
>
> Like in the MediaComponent - Video for example
> (https://ci.apache.org/projects/wicket/apidocs/7.x/org/apache/wicket/markup/html/media/video/Video.html)
> the Image should be able to be configured with a String placed into the src
> attribute.
> Possible constructor to be used for that:
> Image(String id, IModel<?> model, String... srcs)
> As of comments the requirement changed a bit and a new implementation
> "ExternalImage" is the target of this ticket:
> http://apache-wicket.1842946.n4.nabble.com/Image-based-on-external-url-model-object-tt4672692.html#none
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
