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.

Sign up for GitHub

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

Jump to bottom

Fixes Image Sources #459

Closed
wants to merge 3 commits into from
Closed

Fixes Image Sources #459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
94c0f49
Enabled Codacy Coverage
manikmagar Mar 3, 2018
aa9d0d7
Upstream merge
manikmagar Apr 18, 2018
375c4a2
Fixes image sources
manikmagar Apr 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions jbake-core/src/main/java/org/jbake/app/ConfigUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ public interface Keys {
* The configured base URI for the hosted content
*/
String SITE_HOST = "site.host";
/**
* Property to define if image paths should be prepended with site.host url
*/
String IMG_PATH_PREPEND_HOST = "img.path.prepend.host";
}

private final static Logger LOGGER = LoggerFactory.getLogger(ConfigUtil.class);
Expand Down
22 changes: 13 additions & 9 deletions jbake-core/src/main/java/org/jbake/util/HtmlUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,12 @@ public class HtmlUtil {
public static void fixImageSourceUrls(Map<String, Object> fileContents, CompositeConfiguration config){

String htmlContent = fileContents.get(Attributes.BODY).toString();

boolean prependSiteHost = config.getBoolean(Keys.IMG_PATH_PREPEND_HOST);

String siteHost = config.getString(Keys.SITE_HOST);

String rootPath = fileContents.get(Attributes.ROOTPATH).toString();

String uri = fileContents.get(Attributes.URI).toString();

Expand All @@ -56,21 +60,21 @@ public static void fixImageSourceUrls(Map<String, Object> fileContents, Composit
for (Element img : allImgs) {
String source = img.attr("src");

if(source.startsWith("./")){
// image relative to current content is specified,
// lets add current url to it.
source = source.replaceFirst("./", uri);
}

// Now add the root path
if(!source.startsWith("http://")
&& !source.startsWith("https://")){

if(!source.startsWith("/")){
source = rootPath + uri + source.replaceFirst("./", "");
}

if (!siteHost.endsWith("/") && !source.startsWith("/")) siteHost = siteHost.concat("/");

if(prependSiteHost) {
source = siteHost + source;
}

String fullUrl = siteHost + source;

img.attr("src", fullUrl);
img.attr("src", source);

}
}
Expand Down
2 changes: 2 additions & 0 deletions jbake-core/src/main/resources/default.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,5 @@ uri.noExtension=false
uri.noExtension.prefix=
# String used to separate the header from the body
header.separator=~~~~~~
# Prepend site.host to image paths
img.path.prepend.host=true
46 changes: 31 additions & 15 deletions jbake-core/src/test/java/org/jbake/util/HtmlUtilTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public void shouldNotAddBodyHTMLElement(){
Map<String, Object> fileContent = new HashMap<String, Object>();
fileContent.put(Attributes.ROOTPATH, "../../../");
fileContent.put(Attributes.URI, "blog/2017/05/first_post.html");
fileContent.put(Attributes.BODY, "<div> Test <img src='blog/2017/05/first.jpg' /></div>");
fileContent.put(Attributes.BODY, "<div> Test <img src='/blog/2017/05/first.jpg' /></div>");

HtmlUtil.fixImageSourceUrls(fileContent, config);

Expand All @@ -40,34 +40,50 @@ public void shouldNotAddBodyHTMLElement(){

}

@Test
public void shouldNotAddSiteHost(){
Map<String, Object> fileContent = new HashMap<String, Object>();
fileContent.put(Attributes.ROOTPATH, "../../../");
fileContent.put(Attributes.URI, "blog/2017/05/first_post.html");
fileContent.put(Attributes.BODY, "<div> Test <img src='./first.jpg' /></div>");
config.setProperty(ConfigUtil.Keys.IMG_PATH_PREPEND_HOST,false);

HtmlUtil.fixImageSourceUrls(fileContent, config);

String body = fileContent.get(Attributes.BODY).toString();

assertThat(body).contains("src=\"../../../blog/2017/05/first.jpg\"");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only work for the first_post.html file. if it is an post shown with content on the index page the url would be wrong openening the file directly from a browser. on the index page it should be blog/2017/05/first.jpg.


}

@Test
public void shouldAddRootpath(){
public void shouldAddContentPath(){
Map<String, Object> fileContent = new HashMap<String, Object>();
fileContent.put(Attributes.ROOTPATH, "../../../");
fileContent.put(Attributes.URI, "blog/2017/05/first_post.html");
fileContent.put(Attributes.BODY, "<div> Test <img src='blog/2017/05/first.jpg' /></div>");
fileContent.put(Attributes.BODY, "<div> Test <img src='./first.jpg' /></div>");

HtmlUtil.fixImageSourceUrls(fileContent, config);

String body = fileContent.get(Attributes.BODY).toString();

assertThat(body).contains("src=\"http://www.jbake.org/blog/2017/05/first.jpg\"");
assertThat(body).contains("src=\"http://www.jbake.org/../../../blog/2017/05/first.jpg\"");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm. I would have expected the url without the Attributes.ROOTPATH here.


}

@Test
public void shouldAddContentpath(){
public void shouldAddContentPathForCurrentDirectory(){
Map<String, Object> fileContent = new HashMap<String, Object>();
fileContent.put(Attributes.ROOTPATH, "../../../");
fileContent.put(Attributes.URI, "blog/2017/05/first_post.html");
fileContent.put(Attributes.BODY, "<div> Test <img src='./first.jpg' /></div>");
fileContent.put(Attributes.BODY, "<div> Test <img src='first.jpg' /></div>");

HtmlUtil.fixImageSourceUrls(fileContent, config);

String body = fileContent.get(Attributes.BODY).toString();
assertThat(body).contains("src=\"http://www.jbake.org/blog/2017/05/first.jpg\"");

assertThat(body).contains("src=\"http://www.jbake.org/../../../blog/2017/05/first.jpg\"");

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here. it's a strange looking url. as long as the site.host is present do we really need to append the rootpath here? the uri is relative to the website root already, isn't it?

}

@Test
Expand All @@ -86,12 +102,12 @@ public void shouldNotAddRootPath(){
}

@Test
public void shouldAddRootPathForNoExtension(){
public void shouldNotAddRootPathForNoExtension(){
Map<String, Object> fileContent = new HashMap<String, Object>();
fileContent.put(Attributes.ROOTPATH, "../../../");
fileContent.put(Attributes.URI, "blog/2017/05/first_post.html");
fileContent.put(Attributes.NO_EXTENSION_URI, "blog/2017/05/first_post/");
fileContent.put(Attributes.BODY, "<div> Test <img src='blog/2017/05/first.jpg' /></div>");
fileContent.put(Attributes.BODY, "<div> Test <img src='/blog/2017/05/first.jpg' /></div>");

HtmlUtil.fixImageSourceUrls(fileContent, config);

Expand All @@ -113,7 +129,7 @@ public void shouldAddContentPathForNoExtension(){

String body = fileContent.get(Attributes.BODY).toString();

assertThat(body).contains("src=\"http://www.jbake.org/blog/2017/05/first.jpg\"");
assertThat(body).contains("src=\"http://www.jbake.org/../../../blog/2017/05/first.jpg\"");
}

@Test
Expand Down