Skip to content

Commit

Permalink
make decision about preview on the server instead of the client in js
Browse files Browse the repository at this point in the history
  • Loading branch information
domoritz committed Sep 21, 2012
1 parent 9e0d72c commit 66afc06
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 52 deletions.
22 changes: 14 additions & 8 deletions ckan/lib/helpers.py
Expand Up @@ -1319,13 +1319,12 @@ def resource_preview(resource, pkg_id):

format_lower = resource['format'].lower()
directly = False

url = url_for(controller='package', action='resource_datapreview',
resource_id=resource['id'], id=pkg_id, qualified=True)
url = ''
no_preview = False

if resource.get('datastore_active') or format_lower in ['csv', 'xls', 'tsv']:
#default
pass
url = url_for(controller='package', action='resource_datapreview',
resource_id=resource['id'], id=pkg_id, qualified=True)
elif format_lower == 'pdf':
url = url_for(controller='package', action='resource_pdfpreview',
resource_id=resource['id'], id=pkg_id, qualified=True)
Expand All @@ -1335,9 +1334,16 @@ def resource_preview(resource, pkg_id):
directly = True
url = resource['url']
else:
log.warn('not handler for {}'.format(resource['format']))

return snippet("package/snippets/data_preview.html", embed=directly, resource_url=url)
log.info('no handler for {}'.format(resource['format']))
no_preview = True

return snippet(
"package/snippets/data_preview.html",
embed=directly,
resource_url=url,
no_preview=no_preview,
resource_type=resource['format']
)


# these are the functions that will end up in `h` template helpers
Expand Down
36 changes: 2 additions & 34 deletions ckan/public/base/datapreview/datapreview.js
Expand Up @@ -211,13 +211,10 @@ CKAN.DataPreview = function ($, my) {

}

// 4 situations
// 3 situations
// a) something was posted to the datastore - need to check for this
// b) csv or xls (but not datastore)
// c) can be treated as plain text
// d) none of the above but worth iframing (assumption is
// that if we got here (i.e. preview shown) worth doing
// something ...)
resourceData.formatNormalized = my.normalizeFormat(resourceData.format);

resourceData.url = my.normalizeUrl(resourceData.url);
Expand Down Expand Up @@ -293,35 +290,6 @@ CKAN.DataPreview = function ($, my) {
my.showPlainTextData(data);
});
}
else if (resourceData.formatNormalized in {'html':'', 'htm':''}
|| resourceData.url.substring(0,23)=='http://docs.google.com/') {
// we displays a fullscreen dialog with the url in an iframe.
my.$dialog.empty();
var el = $('<iframe></iframe>');
el.attr('src', resourceData.url);
el.attr('width', '100%');
el.attr('height', '100%');
my.$dialog.append(el);
}
// images
else if (resourceData.formatNormalized in {'png':'', 'jpg':'', 'gif':''}
|| resourceData.resource_type=='image') {
// we displays a fullscreen dialog with the url in an iframe.
my.$dialog.empty();
var el = $('<img />');
el.attr('src', resourceData.url);
el.css('max-width', '100%');
el.css('border', 'solid 4px black');
my.$dialog.append(el);
}
else {
// Cannot reliably preview this item - with no mimetype/format information,
// can't guarantee it's not a remote binary file such as an executable.
my.showError({
title: CKAN.Strings.previewNotAvailableForDataType + resourceData.formatNormalized,
message: ''
});
}
};

// Public: Requests the formatted resource data from the webstore and
Expand Down Expand Up @@ -404,7 +372,7 @@ CKAN.DataPreview = function ($, my) {
} else {
return url;
}
}
};

// Public: Escapes HTML entities to prevent broken layout and XSS attacks
// when inserting user generated or external content.
Expand Down
4 changes: 2 additions & 2 deletions ckan/public/base/datapreview/pdfpreview.js
Expand Up @@ -23,10 +23,10 @@ CKAN.PdfPreview = function ($, pdf, my) {
my.loadPreview = function(resourceData) {
var params = {
file: resourceData['url']
}
};

pdf(params);
}
};

// Export the CKANEXT object onto the window.
$.extend(true, window, {CKANEXT: {}});
Expand Down
24 changes: 16 additions & 8 deletions ckan/templates/package/snippets/data_preview.html
@@ -1,10 +1,18 @@
<div class="module-content">
{% if embed %}
{# images can be embedded directly #}
<img class="ckanext-datapreview-direct-embed" src="{{ resource_url }}"></img>
{% else %}
<iframe src="{{ resource_url }}" class="ckanext-datapreview-iframe" frameborder="0" scrolling="auto" width="100%" data-module="data-viewer">
<p>Your browser does not support iframes.</p>
</iframe>
{% endif %}
{% if no_preview %}
<div class="alert alert-block">
<button type="button" class="close" data-dismiss="alert">×</button>

This comment has been minimized.

Copy link
@tobes

tobes Sep 21, 2012

Contributor

you've not indented this block - I'm being picky

This comment has been minimized.

Copy link
@domoritz

domoritz Sep 21, 2012

Author Contributor

Tabs vs spaces and not indented ;-) I fixed it.

<h4>{{_('Preview not available')}} </h4>
{{_('No handler defined for data type:')}} <code>{{ resource_type }}</code>

This comment has been minimized.

Copy link
@tobes

tobes Sep 21, 2012

Contributor

This is not properly translatable as it assumes that the type comes after the error message may be in some languages it could be translated as for data type {type} no handler

we want something more like _('No handler defined for data type: %{type}s').format(type=resource_type) if you want the code you could add it and wrap in the literal but as the resource_type is I believe user setable it would be a security issue so probably not worth the effort of escaping it first

Also the error should be human (non techy) readable so something like 'this resource cannot be previewed.' may be better and removes the issue. as this is in the

maybe just remove this line

jinja2 {% trans %} tags allow the passing of params which may be helpful in cases like this

</div>
{% else %}
{% if embed %}
{# images can be embedded directly #}
<img class="ckanext-datapreview-direct-embed" src="{{ resource_url }}"></img>
{% else %}
<iframe src="{{ resource_url }}" class="ckanext-datapreview-iframe" frameborder="0" scrolling="auto" width="100%" data-module="data-viewer">
<p>{{_('Your browser does not support iframes.')}}</p>
</iframe>
{% endif %}
{% endif %}
</div>

0 comments on commit 66afc06

Please sign in to comment.