Fix #310 - diff highlighter breaks on some strings #320
Conversation
Strings like “constructor” occur as properties on JavaScript objects by default: ``` var x = new Object(); x[“constructor”] // Object ```
Fix #310 - diff highlighter breaks on some strings
|
@dgreensp absolutely the right fix for this issue! 👍 I wonder why I have not ran into that issue since I already use this since 9 month or so. But I'm not that much into JS any more... |
|
Happy to help! I write JavaScript code all day, so I ran into this bug on The cool thing was I didn't even have to rebuild GitX to test the fix, I It's amusing that John Resig himself wrote this bug: On Thursday, May 15, 2014, Mathias Leppich notifications@github.com wrote:
|
|
Maybe @jeresig wants to know about this – eh? The first thing I did was searching for a diff algorithm already implemented in JavaScript and John's implementation was a perfect starting point. Even-though it was written almost 9 years ago... ;-) |
|
There's still a bug, an even more subtle one. The word "proto" in the input string will cause the assignment The solution is to put a hasProp check at the top of the for (var i in ns) On Friday, May 16, 2014, Mathias Leppich notifications@github.com wrote:
|
Fix for rowanj#310. ``` ... message = "TypeError: 'undefined' is not an object (evaluating 'ns[ n[i] ].rows.push')"; sourceURL = "file:///Applications/GitX.app/Contents/Resources/html/lib/diffHighlighter.js"; ``` I turns out that the `inlinediff.diff()` method had issues caused by the use of Objects as HashMap. That's why @dgreensp added a fix using `Object.prototype.hasOwnProperty` (see rowanj#320). However this solution still fails when the key becomes `"__proto__"`, for some reason. A simpler, saver and yet more efficient fix to the root issue is to just prefix the keys. e.g. `'"' + key`.
|
Sounds good! On Sun, May 18, 2014 at 11:53 PM, Mathias Leppich
|
Strings like “constructor” occur as properties on JavaScript objects by default: