Bug 20660

Summary:	Improved error reporting of 'ErrOpenFile' errors
Product:	poppler	Reporter:	Eric Toombs <ewtoombs>
Component:	glib frontend	Assignee:	poppler-bugs <poppler-bugs>
Status:	RESOLVED FIXED	QA Contact:
Severity:	enhancement
Priority:	medium	CC:	ewtoombs
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	improves error reporting in poppler-glib this version leaves the multicase behaviour alone for the most part (see comment). minor adjustments, added commentary

Description Eric Toombs 2009-03-14 06:22:02 UTC

Created attachment 23850 [details] [review]
improves error reporting in poppler-glib

I modified poppler so that if 'ErrOpenFile' occurs from running poppler_document_new_from_file(uri, password, error), the resulting GError will be a G_FILE_ERROR with the appropriate code and message for the circumstances. It is now possible, for instance, for a program using poppler-glib to distinguish 'file not found', 'no permission', 'is a directory', etc. errors.

I started from the git repository today and I've come up with the attached patch.

Comment 1 Eric Toombs 2009-03-14 06:35:18 UTC

This patch also gets rid of what I think many would consider undesirable behaviour allowing, to some extent, case-insensitive matching of file names. It used to try opening, for example, "fileName.Pdf", then failing that try opening "filename.pdf", then failing that try opening "FILENAME.PDF". This attempt at matching the case doesn't make sense in a case-sensitive file system. Also, it would try to open "filename.pdf" without even checking the reason why opening "fileName.Pdf" had failed in the first place, meaning that it could have initially been a problem with file permissions.

Comment 2 Albert Astals Cid 2009-03-14 11:47:57 UTC

Do not remove the multicase behaviour, if you want to propose that, send a separate patch in a separate bug.

Comment 3 Eric Toombs 2009-03-14 15:57:01 UTC

Created attachment 23865 [details] [review]
this version leaves the multicase behaviour alone for the most part (see comment).

This one is the same as the last one but with the asinine multicase behaviour reincorporated and made somewhat more sane. This time, it only tries a different name if the error returned is ENOENT (no such file or directory).

Comment 4 Albert Astals Cid 2009-03-21 10:48:37 UTC

Carlos what do you say about the glib side of the patch? I'm thinking of commiting it to trunk only btw

Comment 5 Eric Toombs 2009-03-21 16:45:44 UTC

Created attachment 24125 [details] [review]
minor adjustments, added commentary

This patch follows immediately after the patch that leaves the multicase behaviour alone. I made some minor adjustments and added some commentary.

By the way, Albert, what do you mean by 'trunk'. I thought that was a subversion thing.

Comment 6 Albert Astals Cid 2009-03-22 08:18:50 UTC

I'm a subversion man ;-) s/trunk/master, that is the development branch, not the stable (0.10.x) one

BTW the patch is a bit wrong, the error line should be filename->getCString and not fn->getCString() but no need to post a new patch i'll fix it when commiting once Carlos says the glib part is ok.

Comment 7 Carlos Garcia Campos 2009-03-22 08:22:20 UTC

(In reply to comment #4)
> Carlos what do you say about the glib side of the patch? I'm thinking of
> commiting it to trunk only btw
> 

looks good to me.

Comment 8 Albert Astals Cid 2009-03-22 14:50:56 UTC

Commited

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.