Page MenuHomePhabricator

Implement a validation check for ISBN links
Closed, ResolvedPublic


Author: avarab

Currently we detect ISBN links by looking for strings like (regex) "ISBN
([0123456789-Xx]+) in the text and work with that. However the ISBN standard
comes with a built in checksum algorithm that allows one to see if the number in
question is actually a conforming number or just a bunch of gibberish.

We should check if that's the case (gibberish) and not give it the special "ISBN

The algorithm is as follows:

  1. 1. Take an ISBN number like ISBN 0-596-00287-4 (for [[Free as in Freedom]])
  2. 2. Split the numbers into the ISBN number and the check digit (0-596-00287 and 4)
  3. 4. Multiply each ISBN digit by its place in the number sequance (nondigits

omitted) counting from 1

if (1*0 + 2*5 + 3*9 + 4*6 + 5*0 + 6*0 + 7*2 + 8*8 + 9*7) mod 11 = 4 (the check

digit) the number is valid, else it's invalid.

Version: unspecified
Severity: enhancement



Related Objects


Event Timeline

bzimport raised the priority of this task from to Lowest.Nov 21 2014, 8:31 PM
bzimport set Reference to bz2391.

avarab wrote:

ISO 2108:1992, revision of third edition (draft)


avarab wrote:

Moved the severity back to minor since this is not really an enhancement, we
currently accept invalid ISBN links and are thus not ISO 2108 conforming.

gangleri wrote:

Beside 10 digit ISBN numbers the validation should handle also 13 digit ISBN numbers.

Best regards Reinhardt [[user:Gangleri]]

Created attachment 4848
Adds new isValidISBN function to SpecialBooksources

attachment isbn_check.patch ignored as obsolete

I just attached a patch with a PHP function I wrote to do validation of both ISBN 10 and ISBN 13. Now, this just needs to be called from an appropriate place, such as magicLinkCallback in Parser.php.

The ISBN validator is checked in isolation, but I have not yet attempted to integrate it.

Committed in r44514 and now used to add a warning on Special:Booksources when given invalid input.

This could be called from the parser as well, but I'm a little hesitant just yet. Parser tests currently contain some bogus ISBN numbers which need to be replaced with legit ones... there's also the possibility that some books really are published with bad ISBNs... on the other hand, Amazon won't let you search for them. :)
Plus the feedback as you're hitting preview might be nice... but it's also not clear what's wrong -- that is, that it's an invalid number rather than that you just didn't do something right. Do we want better feedback?

I think it would be nice to have in the parser as well. How about the error "The checksum of this ISBN number is invalid." Seems clear enough, and should not make people think their syntax is wrong. I don't think it's ideal for the parser to link broken numbers (and of course, tests should be fixed).

Well, that might be pretty annoying if you're just trying to give a sample of ISBN formatting without actually intending to have a valid number... Could use <nowiki> there, but dunno if that's just going to be uglier than not. :(

It takes about 15 seconds to find a valid ISBN to use as an example (Google [ISBN example]). I don't see that as a reason not to have a warning.

Comment on attachment 4848
Adds new isValidISBN function to SpecialBooksources

Marking as obsolete, code is already in core, and presumably has been for some time

wicke wrote:

I also agree with Brion's hesitation to force all ISBN links to be valid in the parser. While it might give some welcome early feedback to editors it would also complicate other situations unnecessarily, so it is not clear to be a win overall.