{"action":"created","issue":{"url":"https://api.github.com/repos/mozilla/pdf.js/issues/6792","labels_url":"https://api.github.com/repos/mozilla/pdf.js/issues/6792/labels{/name}","comments_url":"https://api.github.com/repos/mozilla/pdf.js/issues/6792/comments","events_url":"https://api.github.com/repos/mozilla/pdf.js/issues/6792/events","html_url":"https://github.com/mozilla/pdf.js/pull/6792","id":123556544,"number":6792,"title":"Implement support for Popup annotations","user":{"login":"timvandermeij","id":1993262,"avatar_url":"https://avatars.githubusercontent.com/u/1993262?v=3","gravatar_id":"","url":"https://api.github.com/users/timvandermeij","html_url":"https://github.com/timvandermeij","followers_url":"https://api.github.com/users/timvandermeij/followers","following_url":"https://api.github.com/users/timvandermeij/following{/other_user}","gists_url":"https://api.github.com/users/timvandermeij/gists{/gist_id}","starred_url":"https://api.github.com/users/timvandermeij/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/timvandermeij/subscriptions","organizations_url":"https://api.github.com/users/timvandermeij/orgs","repos_url":"https://api.github.com/users/timvandermeij/repos","events_url":"https://api.github.com/users/timvandermeij/events{/privacy}","received_events_url":"https://api.github.com/users/timvandermeij/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/mozilla/pdf.js/labels/4-annotations","name":"4-annotations","color":"DDDDDD"}],"state":"open","locked":false,"assignee":{"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},"milestone":null,"comments":10,"created_at":"2015-12-22T21:10:24Z","updated_at":"2015-12-22T21:46:32Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/mozilla/pdf.js/pulls/6792","html_url":"https://github.com/mozilla/pdf.js/pull/6792","diff_url":"https://github.com/mozilla/pdf.js/pull/6792.diff","patch_url":"https://github.com/mozilla/pdf.js/pull/6792.patch"},"body":"We already had most code ready for supporting Popup annotations, but it was only available for Text annotations. This patch implements the separation between Text (or any other annotation type for that matter) and Popup annotations. According to the specification they are two very separate entities and should thus be implemented that way.\r\n\r\nThe main reason for this change is because Popup annotations are used with many other annotation types too. In order to support those, we first need to have proper support for Popup annotations. With this patch in place, implementing other annotation types will be much easier.\r\n\r\nA consequence of this patch is that the Text annotation code is greatly simplified. Essentially, a Text annotation is only a container with an image. The Popup annotation takes care of the rest.\r\n\r\nFinally, this change allows for a small bit of refactoring. We no longer need a separate wrapper as the Popup annotation has its own container, which acts as the wrapper. This means we can also remove the strange sanity check that belonged with it.\r\n\r\n@Snuffleupagus Would you be willing to review this patch? If not, feel free to unassign yourself. The test case from this PR can be used for testing this patch, as well as the PDF file from #6230, which is nice because its Popup annotation has a color. Notice that the warnings about unsupported Popup annotations in the console are gone after this patch."},"comment":{"url":"https://api.github.com/repos/mozilla/pdf.js/issues/comments/166739182","html_url":"https://github.com/mozilla/pdf.js/pull/6792#issuecomment-166739182","issue_url":"https://api.github.com/repos/mozilla/pdf.js/issues/6792","id":166739182,"user":{"login":"timvandermeij","id":1993262,"avatar_url":"https://avatars.githubusercontent.com/u/1993262?v=3","gravatar_id":"","url":"https://api.github.com/users/timvandermeij","html_url":"https://github.com/timvandermeij","followers_url":"https://api.github.com/users/timvandermeij/followers","following_url":"https://api.github.com/users/timvandermeij/following{/other_user}","gists_url":"https://api.github.com/users/timvandermeij/gists{/gist_id}","starred_url":"https://api.github.com/users/timvandermeij/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/timvandermeij/subscriptions","organizations_url":"https://api.github.com/users/timvandermeij/orgs","repos_url":"https://api.github.com/users/timvandermeij/repos","events_url":"https://api.github.com/users/timvandermeij/events{/privacy}","received_events_url":"https://api.github.com/users/timvandermeij/received_events","type":"User","site_admin":false},"created_at":"2015-12-22T21:46:32Z","updated_at":"2015-12-22T21:46:32Z","body":"It seems that it is also possible for Text annotations to *not* have an associated Popup annotation (which is the case in that PDF file), in which case the Text annotation needs to provide the popup and not the Popup annotation. This case can be discovered by checking if the Text annotation has a `Popup` dictionary element. If so, the Popup annotation takes care of the popup and otherwise the Text annotation should do that. Thanks a lot, Adobe... :(\r\n\r\nPoints to address:\r\n- [ ] Solve test failures above and fix popup rendering for the tests\r\n- [ ] Handle absence of Popup annotations (factor out the popup code to somewhere else and let both Popup and other annotations access that)\r\n\r\nI will look into these issues and let you know when they are addressed. This will probably also be after Christmas."}}