120334 2013-08-26 17:25:22 -0700 Web Inspector: Column Breakpoint not working, may be off by 1 2013-08-27 07:12:31 -0700 1 1 1 Unclassified WebKit Web Inspector 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck commit-queue ddkilzer graouts joepeck timothy webkit-bug-importer oldest_to_newest 921245 0 joepeck 2013-08-26 17:25:22 -0700 * STEPS TO REPRODUCE 1. Inspect <http://bogojoker.com/shell/> 2. Set breakpoint on pretty printed easySlider.min.js line 19 "var options = …" 3. Reload the page => Expected breakpoint to be hit, it was not * NOTES • Frontend is setting a breakpoint on { lineNumber:1, columnNumber:274 }. However, it should probably be 275. I think 274 (one based) is a ";". • I think this is what was causing issues with "Continue to here". 921246 1 webkit-bug-importer 2013-08-26 17:25:31 -0700 <rdar://problem/14840678> 921264 2 joepeck 2013-08-26 18:09:17 -0700 Nope, I take this back. This is a 0-based number, so the index is correct. In the line: "(function($){$.fn.easySlider=function(options){var defaults={prevId:'prevBtn',nextId:'nextBtn',prevText:'Previous',nextText:'Next',autogeneratePagination:false,orientation:'',speed:800,autoplayDuration:0,restartDuration:2500,loop:false,hoverPause:0,hover:false,easing:null};var options=…" 0-based index 274 is the "v" in "var options" as expected. In ScriptDebugServer::hasBreakpoint I'm seeing line:1, column:275. Now to investigate why these disagree. 921277 3 joepeck 2013-08-26 18:33:20 -0700 I think this is because JSC switched to 1 based column numbers and we're still assuming 0 based in ScriptDebugServer. For example: void ScriptDebugServer::updateCallFrameAndPauseIfNeeded(const DebuggerCallFrame& debuggerCallFrame, intptr_t sourceID, int lineNumber, int columnNumber) { ... TextPosition textPosition(OrdinalNumber::fromOneBasedInt(lineNumber), OrdinalNumber::fromZeroBasedInt(columnNumber)); ... } This does indeed look like it might be the problem. With a quick test script: var a=1; var b=2; var c=3; var d=a+b+c; console.log(d); 921285 4 joepeck 2013-08-26 18:40:05 -0700 Yep, this fixes the issue. Now to write a test for this so it never regresses! 921304 5 209709 joepeck 2013-08-26 20:26:15 -0700 Created attachment 209709 [PATCH] Proposed Fix Fix and test. The reason that a lot of breakpoints were working currently was that if the breakpoint was set at the start of a line (column 0) we break anywhere on the line. That would be the case for non-pretty printed code, or setting a breakpoint on a line in pretty printed source that happened to be on the start of a line in the non-pretty-printed code. 921492 6 209709 ddkilzer 2013-08-27 06:47:30 -0700 Comment on attachment 209709 [PATCH] Proposed Fix r=me 921502 7 209709 commit-queue 2013-08-27 07:12:28 -0700 Comment on attachment 209709 [PATCH] Proposed Fix Clearing flags on attachment: 209709 Committed r154681: <http://trac.webkit.org/changeset/154681> 921503 8 commit-queue 2013-08-27 07:12:31 -0700 All reviewed patches have been landed. Closing bug. 209709 2013-08-26 20:26:15 -0700 2013-08-27 07:12:28 -0700 [PATCH] Proposed Fix column-fix.diff text/plain 6722 joepeck ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCAzNzY1M2NmLi5iMDkwMmE3IDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VM b2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTMtMDgt MjYgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KKworICAgICAgICBXZWIg SW5zcGVjdG9yOiBDb2x1bW4gQnJlYWtwb2ludCBub3Qgd29ya2luZywgbWF5IGJlIG9mZiBieSAx CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMjAzMzQK KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXcml0ZSBh IHByb3RvY29sIHRlc3QgZm9yIHNldHRpbmcgYSBicmVha3BvaW50IGF0IGEgbGluZTpjb2x1bW4u CisKKyAgICAgICAgKiBpbnNwZWN0b3ItcHJvdG9jb2wvZGVidWdnZXIvY29sdW1uLWJyZWFrcG9p bnQtZXhwZWN0ZWQudHh0OiBBZGRlZC4KKyAgICAgICAgKiBpbnNwZWN0b3ItcHJvdG9jb2wvZGVi dWdnZXIvY29sdW1uLWJyZWFrcG9pbnQuaHRtbDogQWRkZWQuCisKIDIwMTMtMDgtMjYgIFJvYiBC dWlzICA8cndsYnVpc0B3ZWJraXQub3JnPgogCiAgICAgICAgIExvbmVseSBzdG9wIGNyYXNoZXMK ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL2luc3BlY3Rvci1wcm90b2NvbC9kZWJ1Z2dlci9jb2x1 bW4tYnJlYWtwb2ludC1leHBlY3RlZC50eHQgYi9MYXlvdXRUZXN0cy9pbnNwZWN0b3ItcHJvdG9j b2wvZGVidWdnZXIvY29sdW1uLWJyZWFrcG9pbnQtZXhwZWN0ZWQudHh0Cm5ldyBmaWxlIG1vZGUg MTAwNjQ0CmluZGV4IDAwMDAwMDAuLjJkNzYwYjUKLS0tIC9kZXYvbnVsbAorKysgYi9MYXlvdXRU ZXN0cy9pbnNwZWN0b3ItcHJvdG9jb2wvZGVidWdnZXIvY29sdW1uLWJyZWFrcG9pbnQtZXhwZWN0 ZWQudHh0CkBAIC0wLDAgKzEsNyBAQAorQnJlYWtwb2ludHMgRW5hYmxlZAorRm91bmQgPHNjcmlw dD4KK1J1bm5pbmcgdGVzdEZ1bmN0aW9uCitIaXQgQnJlYWtwb2ludCEKK0V2YWx1dGVkIHZhbHVl IG9mIGBhYCAoZXhwZWN0aW5nIG51bWJlciAxKSB3YXM6IHsidHlwZSI6Im51bWJlciIsInZhbHVl IjoxLCJkZXNjcmlwdGlvbiI6IjEifQorRXZhbHV0ZWQgdmFsdWUgb2YgYGJgIChleHBlY3Rpbmcg dW5kZWZpbmVkKSB3YXM6IHsidHlwZSI6InVuZGVmaW5lZCJ9CisKZGlmZiAtLWdpdCBhL0xheW91 dFRlc3RzL2luc3BlY3Rvci1wcm90b2NvbC9kZWJ1Z2dlci9jb2x1bW4tYnJlYWtwb2ludC5odG1s IGIvTGF5b3V0VGVzdHMvaW5zcGVjdG9yLXByb3RvY29sL2RlYnVnZ2VyL2NvbHVtbi1icmVha3Bv aW50Lmh0bWwKbmV3IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMC4uNjEzYjEzNwotLS0g L2Rldi9udWxsCisrKyBiL0xheW91dFRlc3RzL2luc3BlY3Rvci1wcm90b2NvbC9kZWJ1Z2dlci9j b2x1bW4tYnJlYWtwb2ludC5odG1sCkBAIC0wLDAgKzEsNTggQEAKKzxodG1sPgorPGhlYWQ+Cis8 c2NyaXB0IHR5cGU9InRleHQvamF2YXNjcmlwdCIgc3JjPSIuLi8uLi9odHRwL3Rlc3RzL2luc3Bl Y3Rvci1wcm90b2NvbC9yZXNvdXJjZXMvcHJvdG9jb2wtdGVzdC5qcyI+PC9zY3JpcHQ+Cis8c2Ny aXB0Pgord2luZG93Lm9ubG9hZCA9IHJ1blRlc3Q7CitmdW5jdGlvbiB0ZXN0KCkKK3sKKyAgICAv LyBUaGlzIHRlc3Qgc2V0dGluZyBhIGJyZWFrcG9pbnQgb24gbGluZTpjb2x1bW4gaW4gdGhlIDxz Y3JpcHQ+IGJlbG93LgorICAgIC8vIFdlIHNldCBhIGJyZWFrcG9pbnQgYmVmb3JlIGV2YWx1YXRp bmcgYHZhciBiID0gLi4uYCwgYW5kIHZlcmlmeSB0aGF0CisgICAgLy8gYnkgcnVubmluZyBzb21l IGV4cHJlc3Npb25zIGF0IHRoYXQgY2FsbCBmcmFtZS4KKworICAgIC8vIERlYnVnZ2VyLmVuYWJs ZSAtPiBEZWJ1Z2dlci5zZXRCcmVha3BvaW50c0FjdGl2ZQorICAgIEluc3BlY3RvclRlc3Quc2Vu ZENvbW1hbmQoIkRlYnVnZ2VyLmVuYWJsZSIsIHt9LCBmdW5jdGlvbigpIHsKKyAgICAgICAgSW5z cGVjdG9yVGVzdC5zZW5kQ29tbWFuZCgiRGVidWdnZXIuc2V0QnJlYWtwb2ludHNBY3RpdmUiLCB7 YWN0aXZlOiB0cnVlfSwgZnVuY3Rpb24oKSB7CisgICAgICAgICAgICBJbnNwZWN0b3JUZXN0Lmxv ZygiQnJlYWtwb2ludHMgRW5hYmxlZCIpOworICAgICAgICB9KTsKKyAgICB9KTsKKworICAgIC8v IERlYnVnZ2VyLnNjcmlwdFBhcnNlZCAtPiBEZWJ1Z2dlci5zZXRCcmVha3BvaW50IC0+IFJ1bnRp bWUuZXZhbHVhdGUKKyAgICBJbnNwZWN0b3JUZXN0LmV2ZW50SGFuZGxlclsiRGVidWdnZXIuc2Ny aXB0UGFyc2VkIl0gPSBmdW5jdGlvbihtZXNzYWdlT2JqZWN0KQorICAgIHsKKyAgICAgICAgaWYg KC9jb2x1bW4tYnJlYWtwb2ludFwuaHRtbCQvLnRlc3QobWVzc2FnZU9iamVjdC5wYXJhbXMudXJs KSAmJiBtZXNzYWdlT2JqZWN0LnBhcmFtcy5zdGFydExpbmUgPiAxMCkgeworICAgICAgICAgICAg SW5zcGVjdG9yVGVzdC5sb2coIkZvdW5kIDxzY3JpcHQ+Iik7CisgICAgICAgICAgICB2YXIgc2Ny aXB0SWRlbnRpZmllciA9IG1lc3NhZ2VPYmplY3QucGFyYW1zLnNjcmlwdElkOworICAgICAgICAg ICAgdmFyIGxpbmVOdW1iZXIgPSBtZXNzYWdlT2JqZWN0LnBhcmFtcy5zdGFydExpbmUgKyAyOwor ICAgICAgICAgICAgdmFyIGNvbHVtbk51bWJlciA9IDEwOworICAgICAgICAgICAgdmFyIGxvY2F0 aW9uID0ge3NjcmlwdElkOiBzY3JpcHRJZGVudGlmaWVyLCBsaW5lTnVtYmVyOiBsaW5lTnVtYmVy LCBjb2x1bW5OdW1iZXI6IGNvbHVtbk51bWJlcn07CisgICAgICAgICAgICBJbnNwZWN0b3JUZXN0 LnNlbmRDb21tYW5kKCJEZWJ1Z2dlci5zZXRCcmVha3BvaW50Iiwge2xvY2F0aW9uOiBsb2NhdGlv bn0sIGZ1bmN0aW9uKCkgeworICAgICAgICAgICAgICAgIEluc3BlY3RvclRlc3QubG9nKCJSdW5u aW5nIHRlc3RGdW5jdGlvbiIpOworICAgICAgICAgICAgICAgIEluc3BlY3RvclRlc3Quc2VuZENv bW1hbmQoIlJ1bnRpbWUuZXZhbHVhdGUiLCB7ZXhwcmVzc2lvbjogInRlc3RGdW5jdGlvbigpIn0p OworICAgICAgICAgICAgfSk7CisgICAgICAgIH0KKyAgICB9CisKKyAgICAvLyBEZWJ1Z2dlci5w YXVzZWQgLT4gRGVidWdnZXIuZXZhbHVhdGVPbkNhbGxGcmFtZSAtPiBEZWJ1Z2dlci5ldmFsdWF0 ZU9uQ2FsbEZyYW1lIC0+IEVORAorICAgIEluc3BlY3RvclRlc3QuZXZlbnRIYW5kbGVyWyJEZWJ1 Z2dlci5wYXVzZWQiXSA9IGZ1bmN0aW9uKG1lc3NhZ2VPYmplY3QpCisgICAgeworICAgICAgICBJ bnNwZWN0b3JUZXN0LmxvZygiSGl0IEJyZWFrcG9pbnQhIik7CisgICAgICAgIHZhciBjYWxsRnJh bWVJZGVudGlmaWVyID0gbWVzc2FnZU9iamVjdC5wYXJhbXMuY2FsbEZyYW1lc1swXS5jYWxsRnJh bWVJZDsKKyAgICAgICAgSW5zcGVjdG9yVGVzdC5zZW5kQ29tbWFuZCgiRGVidWdnZXIuZXZhbHVh dGVPbkNhbGxGcmFtZSIsIHtjYWxsRnJhbWVJZDogY2FsbEZyYW1lSWRlbnRpZmllciwgZXhwcmVz c2lvbjogImEifSwgZnVuY3Rpb24obWVzc2FnZU9iamVjdCkgeworICAgICAgICAgICAgSW5zcGVj dG9yVGVzdC5sb2coIkV2YWx1dGVkIHZhbHVlIG9mIGBhYCAoZXhwZWN0aW5nIG51bWJlciAxKSB3 YXM6ICIgKyBKU09OLnN0cmluZ2lmeShtZXNzYWdlT2JqZWN0LnJlc3VsdC5yZXN1bHQpKTsKKyAg ICAgICAgICAgIEluc3BlY3RvclRlc3Quc2VuZENvbW1hbmQoIkRlYnVnZ2VyLmV2YWx1YXRlT25D YWxsRnJhbWUiLCB7Y2FsbEZyYW1lSWQ6IGNhbGxGcmFtZUlkZW50aWZpZXIsIGV4cHJlc3Npb246 ICJiIn0sIGZ1bmN0aW9uKG1lc3NhZ2VPYmplY3QpIHsKKyAgICAgICAgICAgICAgICBJbnNwZWN0 b3JUZXN0LmxvZygiRXZhbHV0ZWQgdmFsdWUgb2YgYGJgIChleHBlY3RpbmcgdW5kZWZpbmVkKSB3 YXM6ICIgKyBKU09OLnN0cmluZ2lmeShtZXNzYWdlT2JqZWN0LnJlc3VsdC5yZXN1bHQpKTsKKyAg ICAgICAgICAgICAgICBJbnNwZWN0b3JUZXN0LmNvbXBsZXRlVGVzdCgpOworICAgICAgICAgICAg fSk7CisgICAgICAgIH0pOworICAgIH0KK30KKzwvc2NyaXB0PgorPC9oZWFkPgorPGJvZHk+Cis8 c2NyaXB0Pi8vIExpbmUgMAorZnVuY3Rpb24gdGVzdEZ1bmN0aW9uKCkgey8vIExpbmUgMQordmFy IGEgPSAxO3ZhciBiID0gYSArIDE7Ly8gTGluZSAyCit9Ly8gTGluZSAzCis8L3NjcmlwdD4KKzwv Ym9keT4KKzwvaHRtbD4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCA0YjAyNzI0Li4wZGE5MTBiIDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9n CkBAIC0xLDUgKzEsMjEgQEAKIDIwMTMtMDgtMjYgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJv QGFwcGxlLmNvbT4KIAorICAgICAgICBXZWIgSW5zcGVjdG9yOiBDb2x1bW4gQnJlYWtwb2ludCBu b3Qgd29ya2luZywgbWF5IGJlIG9mZiBieSAxCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0xMjAzMzQKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBKYXZhU2NyaXB0Q29yZSBjaGFuZ2VkIHRvIDEtYmFzZWQgY29s dW1uIG51bWJlcnMgYXQgc29tZSBwb2ludC4gV2UKKyAgICAgICAgbmVlZCB0byB1cGRhdGUgdGhl IFNjcmlwdERlYnVnZ2VyIGFzc3VtcHRpb24gdGhhdCB0aGV5IHdlcmUgMC1iYXNlZC4KKworICAg ICAgICBUZXN0OiBpbnNwZWN0b3ItcHJvdG9jb2wvZGVidWdnZXIvY29sdW1uLWJyZWFrcG9pbnQu aHRtbAorCisgICAgICAgICogYmluZGluZ3MvanMvU2NyaXB0RGVidWdTZXJ2ZXIuY3BwOgorICAg ICAgICAoV2ViQ29yZTo6U2NyaXB0RGVidWdTZXJ2ZXI6OmNyZWF0ZUNhbGxGcmFtZSk6CisgICAg ICAgIChXZWJDb3JlOjpTY3JpcHREZWJ1Z1NlcnZlcjo6dXBkYXRlQ2FsbEZyYW1lQW5kUGF1c2VJ Zk5lZWRlZCk6CisKKzIwMTMtMDgtMjYgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxl LmNvbT4KKwogICAgICAgICBXZWIgSW5zcGVjdG9yOiBXZSBzaG91bGQgcmVnZW5lcmF0ZSBJbnNw ZWN0b3JCYWNrZW5kQ29tbWFuZHMuanMgZm9yIExlZ2FjeSBJbnNwZWN0b3IuanNvbiB2ZXJzaW9u cwogICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTIwMjQy CiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL2pzL1NjcmlwdERlYnVnU2Vy dmVyLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL2pzL1NjcmlwdERlYnVnU2VydmVyLmNw cAppbmRleCA5YmExYWUwLi5hYjBjNGRmIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9iaW5k aW5ncy9qcy9TY3JpcHREZWJ1Z1NlcnZlci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvYmluZGlu Z3MvanMvU2NyaXB0RGVidWdTZXJ2ZXIuY3BwCkBAIC0zOTAsNyArMzkwLDcgQEAgdm9pZCBTY3Jp cHREZWJ1Z1NlcnZlcjo6ZGlzcGF0Y2hGdW5jdGlvblRvTGlzdGVuZXJzKEphdmFTY3JpcHRFeGVj dXRpb25DYWxsYmFjawogCiB2b2lkIFNjcmlwdERlYnVnU2VydmVyOjpjcmVhdGVDYWxsRnJhbWUo Y29uc3QgRGVidWdnZXJDYWxsRnJhbWUmIGRlYnVnZ2VyQ2FsbEZyYW1lLCBpbnRwdHJfdCBzb3Vy Y2VJRCwgaW50IGxpbmVOdW1iZXIsIGludCBjb2x1bW5OdW1iZXIpCiB7Ci0gICAgVGV4dFBvc2l0 aW9uIHRleHRQb3NpdGlvbihPcmRpbmFsTnVtYmVyOjpmcm9tT25lQmFzZWRJbnQobGluZU51bWJl ciksIE9yZGluYWxOdW1iZXI6OmZyb21aZXJvQmFzZWRJbnQoY29sdW1uTnVtYmVyKSk7CisgICAg VGV4dFBvc2l0aW9uIHRleHRQb3NpdGlvbihPcmRpbmFsTnVtYmVyOjpmcm9tT25lQmFzZWRJbnQo bGluZU51bWJlciksIE9yZGluYWxOdW1iZXI6OmZyb21PbmVCYXNlZEludChjb2x1bW5OdW1iZXIp KTsKICAgICBtX2N1cnJlbnRDYWxsRnJhbWUgPSBKYXZhU2NyaXB0Q2FsbEZyYW1lOjpjcmVhdGUo ZGVidWdnZXJDYWxsRnJhbWUsIG1fY3VycmVudENhbGxGcmFtZSwgc291cmNlSUQsIHRleHRQb3Np dGlvbik7CiAgICAgaWYgKG1fbGFzdEV4ZWN1dGVkU291cmNlSWQgIT0gc291cmNlSUQpIHsKICAg ICAgICAgbV9sYXN0RXhlY3V0ZWRMaW5lID0gLTE7CkBAIC00MDQsNyArNDA0LDcgQEAgdm9pZCBT Y3JpcHREZWJ1Z1NlcnZlcjo6dXBkYXRlQ2FsbEZyYW1lQW5kUGF1c2VJZk5lZWRlZChjb25zdCBE ZWJ1Z2dlckNhbGxGcmFtZSYKICAgICBpZiAoIW1fY3VycmVudENhbGxGcmFtZSkKICAgICAgICAg cmV0dXJuOwogCi0gICAgVGV4dFBvc2l0aW9uIHRleHRQb3NpdGlvbihPcmRpbmFsTnVtYmVyOjpm cm9tT25lQmFzZWRJbnQobGluZU51bWJlciksIE9yZGluYWxOdW1iZXI6OmZyb21aZXJvQmFzZWRJ bnQoY29sdW1uTnVtYmVyKSk7CisgICAgVGV4dFBvc2l0aW9uIHRleHRQb3NpdGlvbihPcmRpbmFs TnVtYmVyOjpmcm9tT25lQmFzZWRJbnQobGluZU51bWJlciksIE9yZGluYWxOdW1iZXI6OmZyb21P bmVCYXNlZEludChjb2x1bW5OdW1iZXIpKTsKICAgICBtX2N1cnJlbnRDYWxsRnJhbWUtPnVwZGF0 ZShkZWJ1Z2dlckNhbGxGcmFtZSwgc291cmNlSUQsIHRleHRQb3NpdGlvbik7CiAgICAgcGF1c2VJ Zk5lZWRlZChkZWJ1Z2dlckNhbGxGcmFtZS5keW5hbWljR2xvYmFsT2JqZWN0KCkpOwogfQo=