{"action":"created","issue":{"url":"https://api.github.com/repos/mozilla/pdf.js/issues/7347","repository_url":"https://api.github.com/repos/mozilla/pdf.js","labels_url":"https://api.github.com/repos/mozilla/pdf.js/issues/7347/labels{/name}","comments_url":"https://api.github.com/repos/mozilla/pdf.js/issues/7347/comments","events_url":"https://api.github.com/repos/mozilla/pdf.js/issues/7347/events","html_url":"https://github.com/mozilla/pdf.js/pull/7347","id":156297290,"number":7347,"title":"Slightly refactor the `fontRef` handling in `PartialEvaluator_loadFont` (issue 7403 and issue 7402)","user":{"login":"Snuffleupagus","id":2692120,"avatar_url":"https://avatars.githubusercontent.com/u/2692120?v=3","gravatar_id":"","url":"https://api.github.com/users/Snuffleupagus","html_url":"https://github.com/Snuffleupagus","followers_url":"https://api.github.com/users/Snuffleupagus/followers","following_url":"https://api.github.com/users/Snuffleupagus/following{/other_user}","gists_url":"https://api.github.com/users/Snuffleupagus/gists{/gist_id}","starred_url":"https://api.github.com/users/Snuffleupagus/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Snuffleupagus/subscriptions","organizations_url":"https://api.github.com/users/Snuffleupagus/orgs","repos_url":"https://api.github.com/users/Snuffleupagus/repos","events_url":"https://api.github.com/users/Snuffleupagus/events{/privacy}","received_events_url":"https://api.github.com/users/Snuffleupagus/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/mozilla/pdf.js/labels/1-core","name":"1-core","color":"DDDDDD"}],"state":"open","locked":false,"assignee":{"login":"brendandahl","id":942640,"avatar_url":"https://avatars.githubusercontent.com/u/942640?v=3","gravatar_id":"","url":"https://api.github.com/users/brendandahl","html_url":"https://github.com/brendandahl","followers_url":"https://api.github.com/users/brendandahl/followers","following_url":"https://api.github.com/users/brendandahl/following{/other_user}","gists_url":"https://api.github.com/users/brendandahl/gists{/gist_id}","starred_url":"https://api.github.com/users/brendandahl/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/brendandahl/subscriptions","organizations_url":"https://api.github.com/users/brendandahl/orgs","repos_url":"https://api.github.com/users/brendandahl/repos","events_url":"https://api.github.com/users/brendandahl/events{/privacy}","received_events_url":"https://api.github.com/users/brendandahl/received_events","type":"User","site_admin":false},"milestone":null,"comments":25,"created_at":"2016-05-23T14:42:14Z","updated_at":"2016-06-18T08:09:34Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/mozilla/pdf.js/pulls/7347","html_url":"https://github.com/mozilla/pdf.js/pull/7347","diff_url":"https://github.com/mozilla/pdf.js/pull/7347.diff","patch_url":"https://github.com/mozilla/pdf.js/pull/7347.patch"},"body":"Originally, I was just going to change this code to use `Ref_toString` in a couple more places. When I started reading the code, I figured that it wouldn't hurt to clean up a couple of comments. While doing this, I noticed that the logic for the (rare) `isDict(fontRef)` case could do with a few improvements.\r\n\r\nThere should be no functional changes with this patch, but given the added reference checks, we will now avoid bogus `Ref`s when resolving font aliases. ~~In practice, I don't think the current code has caused issues, but it seems good to clean this up to prevent any future problems.~~ *As issue #7403 shows, there're issues with the current code.*\r\n\r\nNote that the only thing that this patch will change, is the `font.loadedName` in the case where a `fontRef` is a reference *and* the font doesn't have a descriptor. Previously for `fontRef = Ref(4, 0)` we'd get `font.loadedName = 'g_d0_f4_0'`, and with this patch `font.loadedName = g_d0_f4R`, which is actually one character shorted in most cases. (Given that `Ref_toString` contains an optimization for the `gen === 0` case, which is by far the most common `gen` value.)\r\n\r\nIn the already existing fallback case, where the `fontName` is used to when creating the `font.loadedName`, we allow any alphanumeric character. Hence I don't see how (as mentioned above) e.g. `font.loadedName = g_d0_f4R` would be an issue here.\r\n\r\n**Edit:** This also fixes #7403 and #7402."},"comment":{"url":"https://api.github.com/repos/mozilla/pdf.js/issues/comments/226928699","html_url":"https://github.com/mozilla/pdf.js/pull/7347#issuecomment-226928699","issue_url":"https://api.github.com/repos/mozilla/pdf.js/issues/7347","id":226928699,"user":{"login":"pdfjsbot","id":1036361,"avatar_url":"https://avatars.githubusercontent.com/u/1036361?v=3","gravatar_id":"","url":"https://api.github.com/users/pdfjsbot","html_url":"https://github.com/pdfjsbot","followers_url":"https://api.github.com/users/pdfjsbot/followers","following_url":"https://api.github.com/users/pdfjsbot/following{/other_user}","gists_url":"https://api.github.com/users/pdfjsbot/gists{/gist_id}","starred_url":"https://api.github.com/users/pdfjsbot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pdfjsbot/subscriptions","organizations_url":"https://api.github.com/users/pdfjsbot/orgs","repos_url":"https://api.github.com/users/pdfjsbot/repos","events_url":"https://api.github.com/users/pdfjsbot/events{/privacy}","received_events_url":"https://api.github.com/users/pdfjsbot/received_events","type":"User","site_admin":false},"created_at":"2016-06-18T08:09:34Z","updated_at":"2016-06-18T08:09:34Z","body":"#### From: Bot.io (Linux)\n\n----\n\n# Success\n\nFull output at http://107.21.233.14:8877/e0cf8d88de4824c/output.txt\n\nTotal script time: 27.71 mins\n\n+ **Font tests:** Passed\n+ **Unit tests:** Passed\n+ **Regression tests:** Passed"}}
{"actor":{"display_login":"pdfjsbot"}}