feat(fonts): make name/object null #8
Conversation
src/pdffonts.cc
Outdated
| fontObj->Set(Nan::New("object").ToLocalChecked(), objectObj); | ||
| // Logic taken from pdffonts.cc | ||
| // See: https://cgit.freedesktop.org/poppler/poppler/tree/utils/pdffonts.cc?id=eb1291f86260124071e12226294631ce685eaad6#n207 | ||
| if (fontRef.gen >= 100000) { |
There was a problem hiding this comment.
What does fontRef.gen represent?
Nit: While the link helps, I think it'd be clearer with a comment that describes what this block is doing instead.
There was a problem hiding this comment.
Its the PDF object generation number. I'll include this link for context: http://www.printmyfolders.com/understanding-pdf
src/pdffonts.cc
Outdated
| if (fontRef.gen >= 100000) { | ||
| fontObj->Set(Nan::New("object").ToLocalChecked(), Nan::Null()); | ||
| } else { | ||
| v8::Local<v8::Object> objectObj = Nan::New<v8::Object>(); |
There was a problem hiding this comment.
What does this object represent? Would it be clearer if we gave it a less specific name than just 'object'?
There was a problem hiding this comment.
It represents metadata associated with the PDF object that is the font. object seems to be used commonly, the original utility references the metadata using the term as well.
There was a problem hiding this comment.
Could metadataObj be an alternative then? I'm coming into this late and have zero context so I trust your judgement.
There was a problem hiding this comment.
I think object might be more clearly understood than reference or metadata, even though thats how I've come to understand it. It kind of represents the PDF object, in a way, though it does require context to understand.
What
Why
In preparation for PDF font validation, we should improve how we handle PDFs with undefined font names and invalid objects. These are corner cases but setting them as
nullis better for now than using empty strings or invalid objects.