Page MenuHomePhabricator

isMultiPage() sometimes returns false even though TIFF is always a multipage format
Closed, ResolvedPublic

Description

Author: inductiveload

Description:
The function isMultiPage() sometimes returns false, even though TIFF is a multipage format. If the particular file has only one page, that doesn't change the format from being a multipage format.

From the notes in "include/filerepo/File.php", this function should return "true" for any format which support multipage, irrespective of the number of pages in that particular file:

"Returns 'true' if this file is a type which supports multiple pages, e.g. DJVU or PDF. Note that this may be true even if the file in question only has a single page."

Thus the current behaviour deviates from the documented API. As it is, it is breaking the ProofreadPage extension which uses the isMultiPage() function to test for ability of the <pagelist/> tag to support that file.

Current behaviour

  • isMultiPage() sometimes returns "false", sometimes "true"

Expected behaviour

  • isMultiPage() always returns "true"

Version: unspecified
Severity: normal

Details

Reference
bz31078

Event Timeline

bzimport raised the priority of this task from to Low.Nov 21 2014, 11:47 PM
bzimport set Reference to bz31078.

Hmm... we actually have a TiffHandler in core that *doesn't* handle multiple pages. Should installing the PagedTiffHandler extension really change the return value and behavior on TIFF images that contain only a single image?

Should ProofreadPage actually be checking for multiple-page support here, or should it treat single-page files and non-multi-page files as equivalent?

I didn't even look Tiff.php thinking it was obsoleted by the paged one. The comment in media/Generic.php about isMultiPage() /* True if the type has multi-page capabilities */ or in filerepo/File.php /* Returns 'true' if this file is a type which supports multiple pages */ is quite clear, the base class ask for a different implementation of isMultiPage() in the paged tiff handler, meaning caller code can't handle multi page file in the same way through the generic interface for tiff file than other multipage file format.

Beside that, there is other trouble in the Proofread extension, fixing this one will not make the extension working with tiff, so if Tiff.php is not obsolete nor can be obsoleted, it's not a big trouble actually for the extension, but my comment about the documented base class behavior remains correct. We will talk probably of the other trouble in another bug.

In PagedTiffHandler we treat a multipage image as a single page one if it has only one page. The reason is that single pages don't have page selectors on the image page, which I thought would just be some GUI overhead for the user if there is only one page. Declaring such files as "not multipage" seemed to be an easy way to achieve this. I admit, though, we hadn't thought about other extensions relying on such information. So I suggest we switch to the documented behaviour in PagedTiffHandler and always return true. I have to do a few tests and then I'll check it in.
The GUI issue remains, but I suggest we treat that separately.

Ok, per all info I can find (eg http://www.awaresystems.be/imaging/tiff/faq.html#q3) there's no difference between a single-page and multi-page TIFF other than having multiple pages: the image file directory blocks form a linked list, and there's simply only one for a single-page file.

And it looks to me like the things which deal with multiple-page output in the UI are actually checking pageCount() as well to see if there actually are multiple pages to deal with. So there shouldn't be negative consequences to just flipping it to return true consistently.

Yeps, I tested it on my local wiki with a one page djvu, and there is no selector for one page djvu, I guess it'll work for tiff file too, this was probably fixed after the paged tiff handler was released.

fixed in r98016. All tests (Selenium and PHPUnit) still pass.