GNOME Bugzilla – Bug 686527
Pickup ownCloud accounts from GOA
Last modified: 2013-08-20 14:25:59 UTC
ownCloud accounts offer WebDAV endpoints whose URI is available as the Uri property on the account's org.gnome.OnlineAccounts.Files DBus interface. It would be good if Documents picked up ownCloud accounts.
Lets push this back to 3.10.
Created attachment 252207 [details] [review] lib: support loading non-file:// GVfs URIs
Created attachment 252208 [details] [review] documents: implement OwncloudDocument
Created attachment 252209 [details] [review] miners: Add a JS stub for org.gnome.OnlineMiners.OwncloudMiner
Created attachment 252210 [details] [review] miners: refresh the SkyDrive cache on startup
Created attachment 252211 [details] [review] documents: check the nao:identifier and instantiate a OwncloudDocument
Created attachment 252212 [details] [review] properties: show an appropriate source for OwncloudDocuments
Created attachment 252334 [details] [review] miners: refresh the ownCloud cache on startup Fix commit message: it is ownCloud not SkyDrive.
Review of attachment 252207 [details] [review]: I'd really rather have this work nicely in evince and fix any bugs there rather than in GdPdfLoader. We can accept this as a short-term solution though. ::: src/lib/gd-pdf-loader.c @@ +1124,3 @@ + content_type = g_file_info_get_content_type (info); + if (!content_type_is_native (content_type)) + g_error_free (error); Brace style should be consistent with the rest of the file. @@ +1180,3 @@ else if (job->zpj_entry != NULL) pdf_load_job_from_skydrive (job); + else if (!g_str_has_prefix (job->uri, "file://")) I think this should use g_file_is_native() instead.
Review of attachment 252208 [details] [review]: Looks good
Review of attachment 252209 [details] [review]: OK
Review of attachment 252334 [details] [review]: OK
Review of attachment 252211 [details] [review]: OK
Review of attachment 252212 [details] [review]: Looks good
Created attachment 252419 [details] [review] lib: support loading non-native GVfs URIs
Created attachment 252421 [details] [review] lib: support loading non-native GVfs URIs
(In reply to comment #9) > Review of attachment 252207 [details] [review]: > > I'd really rather have this work nicely in evince and fix any bugs there rather > than in GdPdfLoader. We can accept this as a short-term solution though. Yeah. > ::: src/lib/gd-pdf-loader.c > @@ +1124,3 @@ > + content_type = g_file_info_get_content_type (info); > + if (!content_type_is_native (content_type)) > + g_error_free (error); > > Brace style should be consistent with the rest of the file. Is it ok now? > @@ +1180,3 @@ > else if (job->zpj_entry != NULL) > pdf_load_job_from_skydrive (job); > + else if (!g_str_has_prefix (job->uri, "file://")) > > I think this should use g_file_is_native() instead. Done.
Review of attachment 252421 [details] [review]: Looks good, other than these two cosmetic issues below. ::: src/lib/gd-pdf-loader.c @@ +959,2 @@ pdf_load_job_from_openoffice (job); + } This change isn't needed. @@ +1125,3 @@ + content_type = g_file_info_get_content_type (info); + if (!content_type_is_native (content_type)) + g_error_free (error); I was referring to these braces here, should be in the same line of the if above.
(In reply to comment #18) > Review of attachment 252421 [details] [review]: > > Looks good, other than these two cosmetic issues below. Oops. Sorry for the mix up. Fixed. > @@ +1125,3 @@ > + content_type = g_file_info_get_content_type (info); > + if (!content_type_is_native (content_type)) > + g_error_free (error); I wonder why Splinter keeps showing the 'g_error_free'. It is not present in that snippet. :-/
Created attachment 252431 [details] [review] lib: support loading non-native GVfs URIs
Review of attachment 252431 [details] [review]: Looks good!
Thank you very much for all the reviews.