Allow open local pdf file from FileBrowse UI.

Allow open local pdf file from FileBrowse UI.

Allow local pdf file to be opened if a pdf plugin is available.

BUG=chromium-os:5263
TEST=Verify local pdf file could be opened and viewed by pdf plugin.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=54964

[dhg, 2010-08-04 19:33:49]

LGTM

On Wed, Aug 4, 2010 at 11:20 AM,  <xiyuan@chromium.org> wrote:
> Reviewers: dhg, arv,
>
> Description:
> Allow open local pdf file from FileBrowse UI.
>
> Allow local pdf file to be opened if a pdf plugin is available.
>
> BUG=chromium-os:5263
> TEST=Verify local pdf file could be opened and viewed by pdf plugin.
>
> Please review this at http://codereview.chromium.org/3083015/show
>
> SVN Base: http://src.chromium.org/git/chromium.git
>
> Affected files:
>  M chrome/browser/resources/filebrowse.html
>  M chrome/browser/resources/shared/js/media_common.js
>
>
> Index: chrome/browser/resources/filebrowse.html
> diff --git a/chrome/browser/resources/filebrowse.html
> b/chrome/browser/resources/filebrowse.html
> index
>
786e513002b9351050023988124125b82155be17..70e3bbaa30d9f3fce285ee80ef454047fa19718b
> 100644
> --- a/chrome/browser/resources/filebrowse.html
> +++ b/chrome/browser/resources/filebrowse.html
> @@ -577,6 +577,10 @@ function partial(fn, var_args) {
>   };
>  }
>
> +function supportsPdf() {
> +  return !!navigator.mimeTypes['application/pdf'];
> +}
> +
>  var currentSavedPath = '';
>  var currentNode = -1;
>  var menus = [];
> @@ -1260,7 +1264,7 @@ function showImage(path) {
>   }
>  }
>
> -function showText(path) {
> +function showPath(path) {
>   chrome.send('openNewFullWindow', ['file://' + path]);
>  }
>
> @@ -1389,9 +1393,15 @@ function getFunctionForItem(path, id, isDirectory) {
>   }
>   if (pathIsHtmlFile(path)) {
>     return function() {
> -      showText(path);
> +      showPath(path);
> +    }
> +  }
> +  if (pathIsPdfFile(path) && supportsPdf()) {
> +    return function() {
> +      showPath(path);
>     }
>   }
> +
>   return getUnknownFileTypeHandler();
>  }
>
> @@ -1466,7 +1476,7 @@ function createNewItem(title, path, isDirectory) {
>       menu.appendChild(flickritem);
>       menu.appendChild(emailitem);
>     }
> -    if ((pathIsVideoFile(path) || pathIsAudioFile(path)) &&
> +    if ((pathIsVideoFile(path) || pathIsAudioFile(path)) &&
>          mediaPlayerEnabled) {
>       var enqueueitem = document.createElement('div');
>       enqueueitem.textContent = localStrings.getString('enqueue');
> Index: chrome/browser/resources/shared/js/media_common.js
> diff --git a/chrome/browser/resources/shared/js/media_common.js
> b/chrome/browser/resources/shared/js/media_common.js
> index
>
85288093a49e3cf49b553fe46833c48a90f94663..86b48c87c297a7105059217ed4df2ff56554e8f2
> 100644
> --- a/chrome/browser/resources/shared/js/media_common.js
> +++ b/chrome/browser/resources/shared/js/media_common.js
> @@ -17,3 +17,7 @@ function pathIsImageFile(path) {
>  function pathIsHtmlFile(path) {
>   return /\.(htm|html|txt)$/i.test(path);
>  }
> +
> +function pathIsPdfFile(path) {
> +  return /\.(pdf)$/i.test(path);
> +}
>
>
>

[arv (Not doing code reviews), 2010-08-04 20:20:08]

LGTM

http://codereview.chromium.org/3083015/diff/1/2
File chrome/browser/resources/filebrowse.html (right):

http://codereview.chromium.org/3083015/diff/1/2#newcode581
chrome/browser/resources/filebrowse.html:581: return
!!navigator.mimeTypes['application/pdf'];
return 'application/pdf' in navigator.mimeTypes;

[xiyuan, 2010-08-04 20:51:48]

Thanks for reviewing... will submit shortly.

http://codereview.chromium.org/3083015/diff/1/2
File chrome/browser/resources/filebrowse.html (right):

http://codereview.chromium.org/3083015/diff/1/2#newcode581
chrome/browser/resources/filebrowse.html:581: return
!!navigator.mimeTypes['application/pdf'];
On 2010/08/04 20:20:08, arv wrote:
> return 'application/pdf' in navigator.mimeTypes;

Done.