{"action":"created","issue":{"url":"https://api.github.com/repos/readium/readium-shared-js/issues/157","labels_url":"https://api.github.com/repos/readium/readium-shared-js/issues/157/labels{/name}","comments_url":"https://api.github.com/repos/readium/readium-shared-js/issues/157/comments","events_url":"https://api.github.com/repos/readium/readium-shared-js/issues/157/events","html_url":"https://github.com/readium/readium-shared-js/issues/157","id":57940840,"number":157,"title":"reading location / bookmark contains HTML element reference + in-page visual percentage (instead of character offset)","user":{"login":"danielweck","id":625201,"avatar_url":"https://avatars.githubusercontent.com/u/625201?v=3","gravatar_id":"","url":"https://api.github.com/users/danielweck","html_url":"https://github.com/danielweck","followers_url":"https://api.github.com/users/danielweck/followers","following_url":"https://api.github.com/users/danielweck/following{/other_user}","gists_url":"https://api.github.com/users/danielweck/gists{/gist_id}","starred_url":"https://api.github.com/users/danielweck/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/danielweck/subscriptions","organizations_url":"https://api.github.com/users/danielweck/orgs","repos_url":"https://api.github.com/users/danielweck/repos","events_url":"https://api.github.com/users/danielweck/events{/privacy}","received_events_url":"https://api.github.com/users/danielweck/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/readium/readium-shared-js/labels/bug","name":"bug","color":"f7c6c7"},{"url":"https://api.github.com/repos/readium/readium-shared-js/labels/priority%20high%20[critical]","name":"priority high [critical]","color":"eb6420"},{"url":"https://api.github.com/repos/readium/readium-shared-js/labels/Shared-JS","name":"Shared-JS","color":"bfdadc"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-02-17T15:50:30Z","updated_at":"2015-08-03T22:16:16Z","closed_at":null,"body":"\r\nThe reading location \"bookmark\" that Readium saves contains 2D coordinates attached to a given HTML element, which is a broken design feature given that the visual consistency of markup in reflowable documents cannot be guaranteed from one reading session to the next (custom font size, line spacing, etc.).\r\n\r\nFurthermore, this is an abuse of the intended feature:\r\nhttp://www.idpf.org/epub/linking/cfi/epub-cfi.html#sec-path-terminating-spatial\r\n\"\r\n3.1.6 Spatial Offset (@)\r\n\r\nA path terminating with a leading at sign (@) followed by two colon-separated numbers indicates a 2D spatial position within an image or video. The two numbers represent scaled locations in the x and y axes, and must be in the range 0 to 100 regardless of the image's native or display dimensions (i.e., the upper left is 0:0 and the lower right is 100:100).\r\n\"\r\n\r\nCode references:\r\n\r\nSAVE:\r\n\r\n`bookmarkCurrentPage()` => `getFirstVisibleElementCfi()` => `return cfi + \"@0:\" + foundElement.percentY`\r\n\r\nhttps://github.com/readium/readium-shared-js/blob/develop/js/views/cfi_navigation_logic.js#L603\r\n\r\nhttps://github.com/readium/readium-shared-js/blob/develop/js/views/reflowable_view.js#L719\r\n\r\nhttps://github.com/readium/readium-shared-js/blob/develop/js/views/cfi_navigation_logic.js#L544\r\n\r\n\r\nRESTORE:\r\n\r\n`openBook()` => `openPageRequest.idref` (OPF spine>itemref@idref) + `openPageRequest.elementCfi` (i.e. even index + `@0:Y`, instead of odd character offset) => `openSpineItemElementCfi()`\r\n\r\nhttps://github.com/readium/readium-shared-js/blob/develop/js/views/reader_view.js#L397\r\n\r\nhttps://github.com/readium/readium-shared-js/blob/develop/js/views/reader_view.js#L667\r\n\r\n... => `openPage()` => `CfiNavigationLogic.getPageForElementCfi()`\r\n\r\nhttps://github.com/readium/readium-shared-js/blob/develop/js/views/reflowable_view.js#L386\r\n\r\n... => `getPageForPointOnElement()` which takes (x,y) 2D coordinates\r\n\r\nhttps://github.com/readium/readium-shared-js/blob/develop/js/views/cfi_navigation_logic.js#L573\r\n\r\nhttps://github.com/readium/readium-shared-js/blob/develop/js/views/cfi_navigation_logic.js#L603\r\n"},"comment":{"url":"https://api.github.com/repos/readium/readium-shared-js/issues/comments/127420024","html_url":"https://github.com/readium/readium-shared-js/issues/157#issuecomment-127420024","issue_url":"https://api.github.com/repos/readium/readium-shared-js/issues/157","id":127420024,"user":{"login":"JCCR","id":5132652,"avatar_url":"https://avatars.githubusercontent.com/u/5132652?v=3","gravatar_id":"","url":"https://api.github.com/users/JCCR","html_url":"https://github.com/JCCR","followers_url":"https://api.github.com/users/JCCR/followers","following_url":"https://api.github.com/users/JCCR/following{/other_user}","gists_url":"https://api.github.com/users/JCCR/gists{/gist_id}","starred_url":"https://api.github.com/users/JCCR/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JCCR/subscriptions","organizations_url":"https://api.github.com/users/JCCR/orgs","repos_url":"https://api.github.com/users/JCCR/repos","events_url":"https://api.github.com/users/JCCR/events{/privacy}","received_events_url":"https://api.github.com/users/JCCR/received_events","type":"User","site_admin":false},"created_at":"2015-08-03T22:16:15Z","updated_at":"2015-08-03T22:16:15Z","body":"Sorry, to clarify by \"we\" I meant the company I work for (Evident Point)\nand by \"got the go ahead\" I meant to say that we haven't yet but if we do\nthen it would start on a new branch.\n\nOn Mon, Aug 3, 2015, 12:45 PM Juan Corona <juan.carlos.corona.r@gmail.com>\nwrote:\n\n> I have a branch: `feature/improve_cfi_accuracy`\n>\n> It's an attempt to get text CFIs. You could build off of that, but the\n> branch is stale and there's some messy code with \"outdated\" techniques. We\n> have come up with better methods of accomplishing text CFIs, so when we got\n> the go ahead to contribute that I was going to start again new branch.\n>\n> Therefore I would think it would be better to bring your code in to what's\n> on `develop`\n>\n> On Mon, Aug 3, 2015, 11:50 AM Michael Atwood <notifications@github.com>\n> wrote:\n>\n>> Is there a branch where the text CFIs have been implemented? I have some\n>> internal code where we bolted on text CFIs, but I would like to either\n>> adapt and add it to Readium's cfi_navigation_logic.js or use what is\n>> already in process of being implemented.\n>>\n>> Changing to text based CFIs will be a breaking change, but spatial CFIs\n>> are already broken in most cases.\n>>\n>> —\n>> Reply to this email directly or view it on GitHub\n>> <https://github.com/readium/readium-shared-js/issues/157#issuecomment-127368578>\n>> .\n>>\n>\n"}}