{"action":"created","issue":{"url":"https://api.github.com/repos/jquery/jquery/issues/3448","repository_url":"https://api.github.com/repos/jquery/jquery","labels_url":"https://api.github.com/repos/jquery/jquery/issues/3448/labels{/name}","comments_url":"https://api.github.com/repos/jquery/jquery/issues/3448/comments","events_url":"https://api.github.com/repos/jquery/jquery/issues/3448/events","html_url":"https://github.com/jquery/jquery/issues/3448","id":194939241,"number":3448,"title":"CSS \"hook\" for left and top styles makes no sense","user":{"login":"david-mark","id":709233,"avatar_url":"https://avatars.githubusercontent.com/u/709233?v=3","gravatar_id":"","url":"https://api.github.com/users/david-mark","html_url":"https://github.com/david-mark","followers_url":"https://api.github.com/users/david-mark/followers","following_url":"https://api.github.com/users/david-mark/following{/other_user}","gists_url":"https://api.github.com/users/david-mark/gists{/gist_id}","starred_url":"https://api.github.com/users/david-mark/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/david-mark/subscriptions","organizations_url":"https://api.github.com/users/david-mark/orgs","repos_url":"https://api.github.com/users/david-mark/repos","events_url":"https://api.github.com/users/david-mark/events{/privacy}","received_events_url":"https://api.github.com/users/david-mark/received_events","type":"User","site_admin":false},"labels":[{"id":156210903,"url":"https://api.github.com/repos/jquery/jquery/labels/Offset","name":"Offset","color":"0052cc","default":false}],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2016-12-12T10:31:15Z","updated_at":"2016-12-12T17:54:10Z","closed_at":"2016-12-12T15:18:58Z","body":"<!--\r\nFeature Requests:\r\n  Please read https://github.com/jquery/jquery/wiki/Adding-new-features\r\n  Most features should start as plugins outside of jQuery.\r\n\r\nBug Reports:\r\n  Note that we only can fix bugs in the latest version of jQuery.\r\n  Briefly describe the issue you've encountered\r\n  *  What do you expect to happen?\r\n  *  What acually happens?\r\n  *  Which browsers are affected?\r\n  Provide a *minimal* test case, see https://webkit.org/test-case-reduction/\r\n  Use the latest shipping version of jQuery in your test case!\r\n  We prefer test cases on https://jsbin.com or https://jsfiddle.net\r\n\r\nFrequently Reported Issues:\r\n  * Selectors with '#' break: See https://github.com/jquery/jquery/issues/2824\r\n-->\r\n\r\n### Description ###\r\n\r\nNothing more pointless in browser scripting than trying to compute styles. The `document.defaultView.getComputedStyle` (incorrectly referenced as `window.getComputedStyle` in jQuery) method is a hideously complex black box that virtually never needs to be opened. In twenty years of browser scripting, I've managed to avoid it almost entirely (exception that comes to mind was an HTML editor).\r\n\r\nRegardless, jQuery aspires to fix the whole thing and frequently gets into trouble. Consider this \"hook\" for the `top` and `left` styles:\r\n\r\n```javascript\r\n// Support: Safari <=7 - 9.1, Chrome <=37 - 49\r\n// Add the top/left cssHooks using jQuery.fn.position\r\n// Webkit bug: https://bugs.webkit.org/show_bug.cgi?id=29084\r\n// Blink bug: https://bugs.chromium.org/p/chromium/issues/detail?id=589347\r\n// getComputedStyle returns percent when specified for top/left/bottom/right;\r\n// rather than make the css module depend on the offset module, just check for it here\r\njQuery.each( [ \"top\", \"left\" ], function( i, prop ) {\r\n\tjQuery.cssHooks[ prop ] = addGetHookIf( support.pixelPosition,\r\n\t\tfunction( elem, computed ) {\r\n\t\t\tif ( computed ) {\r\n\t\t\t\tcomputed = curCSS( elem, prop );\r\n\r\n\t\t\t\t// If curCSS returns percentage, fallback to offset\r\n\t\t\t\treturn rnumnonpx.test( computed ) ?\r\n\t\t\t\t\tjQuery( elem ).position()[ prop ] + \"px\" :\r\n\t\t\t\t\tcomputed;\r\n\t\t\t}\r\n\t\t}\r\n\t);\r\n} );\r\n```\r\n\r\nThe cryptically named `support.pixelPosition` flag refers to a test result where a percentage-based `top` style fails to compute to pixel units. This is an issue that dates back to the bad old days of IE 8-, where `currentStyle` was frequently used by libraries as a slightly-off substitute for `getComputedStyle`; it didn't convert *any* units to pixels. jQuery certainly does this in older versions, so the problem should be familiar.\r\n\r\nThis is the part that attempts to fix the issue:\r\n\r\n```javascript\r\n// If curCSS returns percentage, fallback to offset\r\nreturn rnumnonpx.test( computed ) ?\r\n\tjQuery( elem ).position()[ prop ] + \"px\" :\r\n\tcomputed;\r\n```\r\n\r\nIt calls the `position` method instead, which attempts to figure the *offset* of the element from the document origin, which would only be equivalent to the computed `left` and `top` styles in only a small subset of cases.\r\n\r\nFor example, an element with a position style of `absolute` with all parent elements having position: `static` would work. On the other hand, an element with `fixed` position would fail *unless* the document's scroll position is its origin. Furthermore, an element with `relative` position would fail in virtually every case. And so on, this \"hook\" will simply cause browsers demonstrating this quirk to return nonsensical position values in numerous cases (not even considering problems related RTL documents).\r\n\r\nFor those who never bothered to read my \"P is for Position\" primer on the old cinsoft.net site (now retired) or the numerous Usenet articles I authored on this subject (dating back to at least 2008), I'll go over the general solution again.\r\n\r\nIn short:\r\n\r\n1. Save the `left` or `top` style (e.g. `elem.style.left`).\r\n2. Get the offset.\r\n3. Set the `left` or `top` style to `'0'`.\r\n4. Get the offset again.\r\n5. Find the *difference* between the two offsets.\r\n6. Put the old `left` or `top` style back.\r\n\r\nAnd as the `offset` method is just a pointless wrapper for `getBoundingClientRect` (seems to exist only to handle calls passing disembodied elements or elements with a display style of `none`), would advise simply calling `getBoundingClientRect` and documenting that trying to compute position styles (or offsets) for such elements will result in undefined behavior (a recurring theme here). In addition, to let callers know they made a mistake at the earliest possible time, throw an exception in such cases (as opposed to returning zeros).\r\n\r\nHTH\r\n\r\nPS. What about `right` and `bottom` positions? The solution as it sits is only half-finished. Luckily I archived the related primer years ago:\r\n\r\nhttps://github.com/david-mark/My-Library/blob/master/position.html\r\n\r\nPPS. As this is an old issue that surely affects previous jQuery versions, am curious as to how the old versions are kept up to date when bugs like this are fixed.\r\n\r\n### Link to test case ###\r\n\r\nNo test case needed. Logic is clearly incorrect."},"comment":{"url":"https://api.github.com/repos/jquery/jquery/issues/comments/266501594","html_url":"https://github.com/jquery/jquery/issues/3448#issuecomment-266501594","issue_url":"https://api.github.com/repos/jquery/jquery/issues/3448","id":266501594,"user":{"login":"david-mark","id":709233,"avatar_url":"https://avatars.githubusercontent.com/u/709233?v=3","gravatar_id":"","url":"https://api.github.com/users/david-mark","html_url":"https://github.com/david-mark","followers_url":"https://api.github.com/users/david-mark/followers","following_url":"https://api.github.com/users/david-mark/following{/other_user}","gists_url":"https://api.github.com/users/david-mark/gists{/gist_id}","starred_url":"https://api.github.com/users/david-mark/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/david-mark/subscriptions","organizations_url":"https://api.github.com/users/david-mark/orgs","repos_url":"https://api.github.com/users/david-mark/repos","events_url":"https://api.github.com/users/david-mark/events{/privacy}","received_events_url":"https://api.github.com/users/david-mark/received_events","type":"User","site_admin":false},"created_at":"2016-12-12T17:54:10Z","updated_at":"2016-12-12T17:54:10Z","body":"> Then you're just making noise.\r\n\r\nAnd how do you figure that? There are simply no \"hooks\" for right/bottom. All it would take is to search jQuery.js for \"right\" and \"bottom\". One line:\r\n\r\n```javascript\r\nvar cssExpand = [ \"Top\", \"Right\", \"Bottom\", \"Left\" ];\r\n```\r\n\r\nAnd we both know that has nothing to do with the issue at hand. No test case needed. And as you have prematurely closed the ticket, I'm *certainly* not going to write one.\r\n\r\nFurthermore, to address your previous objection:\r\n\r\n> See http://api.jquery.com/position/ (emphasis mine): \"Get the current coordinates of the first element in the set of matched elements, relative to the offset parent.\"\r\n\r\nShouldn't even need to look at the code to realize your inference that this fixes the central issue raised here is false. For example, what about elements with a `position` style of `relative`? How would finding the offset relative to the offset *parent* help with those?\r\n\r\nWill handle a handful a few edge cases that work by coincidence, but should be obvious that the strategy will fail far more often than it works. Do we really need a test to prove this? Regardless, I'm definitely not writing any test cases for closed tickets.\r\n\r\nCan't see the point in closing tickets before they have been discussed to the point where *everyone* understands the issues raised, as well as the proposed solutions. Is it an accounting thing?\r\n\r\nWas told this was the best place to discuss proposed changes and I'm not seeing that at all. What I am seeing is a lot of confused, knee-jerk reactions and complaints that I'm just taking up time here. Easy solution to that. :)"}}
{"actor":{"display_login":"david-mark"}}