GNOME Bugzilla – Bug 759180
Can't open PDF file shared in Google Drive
Last modified: 2016-01-18 12:57:49 UTC
Hopefully this link works: https://drive.google.com/folderview?id=0BxCVOQS3ZymGfmJyY2RKbE5XbVlKNlktVTlwV0lxNEdxd2dzeWZER094cmJPVnMxN1F0Yzg&ths=true&usp=docs_home&ddrp=1# And you should be able to see a bunch of PDF files, which are specifications for touchscreen controllers. I have those appearing in gnome-documents, but it fails to open and I get the spinner forever.
+ Trace 235798
$1 = (gchar *) 0x0 (gdb) list 390 return NULL; 391 } 392 393 /* Get the download URI and create a stream for it */ 394 download_uri = gdata_documents_document_get_download_uri (self, export_format); 395 download_stream = GDATA_DOWNLOAD_STREAM (gdata_download_stream_new (GDATA_SERVICE (service), domain, download_uri, cancellable)); 396 g_free (download_uri); 397 398 return download_stream; 399 }
That looks to me like a bug in libgdata, which shouldn't call gdata_download_stream_new() when the download_uri is NULL, and should return an error at the very least. libgdata-0.17.3-1.fc23.x86_64
Yes, this is a bug in libgdata that I introduced during the port to Drive v2 API (bug 684920).
Created attachment 317699 [details] [review] documents: Fix error handling for unknown formats for document downloads TODO: Need to add unit tests and work out why the ‘pdf’ format is not working for Bastien’s specific case. --- This is a work in progress patch which fixes the crash but not (yet) the underlying problem, or adds any test cases.
Let me know what information you need to be able to debug this.
Review of attachment 317699 [details] [review]: I totally forgot about this patch from you, and wrote the exact same thing. :) The issue here is that we use the exportLinks from JSON to do the format conversion during download. Unfortunately, if you specify "pdf" as the export_format for something that is already a PDF, there won't be any hits in exportLinks and it would return NULL. We could add more logic to gdata_documents_document_get_download_uri to fallback to gdata_entry_get_content_uri in those cases.
Created attachment 319132 [details] [review] documents: Split out the code to get the content type of an entry
Review of attachment 319132 [details] [review]: Looks good apart from these tiny nits; feel free to commit with them fixed. ::: gdata/services/documents/gdata-documents-utils.c @@ +69,3 @@ + * Returns the content type of @entry, if any. + * + * Return value: content type of @entry, NULL otherwise (nullable) s/NULL/%NULL/
Created attachment 319135 [details] [review] documents: Handle cases where export format is same as the original
Review of attachment 319135 [details] [review]: Could do with a comment explaining why it’s necessary (basically what you put in comment #5).
Created attachment 319244 [details] [review] documents: Split out the code to get the content type of an entry Pushed to master after fixing the documentation markup and annotations.
Created attachment 319245 [details] [review] documents: Handle cases where export format is same as the original
Review of attachment 319245 [details] [review]: ++
(In reply to Philip Withnall from comment #12) > Review of attachment 319245 [details] [review] [review]: > > ++ Pushed to master. While tthis particular bug should now be fixed, I think that it would be useful to have your patch too. I'll let you decide its fate. :)
Attachment 317699 [details] pushed as dc9d85d - documents: Fix error handling for unknown formats for document downloads