Adding support for the <input> element and fixing scrollbar-related bugs.

[mistic100]

thanks for your changes but two problems :

• serious performance issues with textareas with a scrollbar (unable to scroll or select text and CPU usage jumps to 20%)
• input highlight not compatible with scrolling content (when the content is longer than the input)

[rinogo]

Thanks, mistic100! I’ll take a look. I did notice some performance issues (especially in FF); what browser were you doing your testing in?

Also, I tested "long input" in Safari, Firefox, and Chrome, and didn’t experience any issues. Can you tell me which browser(s) you’re having troubles with? Also, in the interest of reproducing the problem: Are you doing testing with the demo case on demo.htm? What is the exact issue you’re seeing? (Expected behavior vs actual behavior)

-Rich

On Dec 20, 2013, at 2:33 PM, mistic100 notifications@github.com wrote:

thanks for your changes but two problems :

serious performance issues with textareas with a scrollbar (unable to scroll or select text and CPU usage jumps to 20%) input highlight not compatible with scrolling content (when the content is longer than the input) — Reply to this email directly or view it on GitHub.

[mistic100]

These issues only occur with FF, no problem with Chrome or IE I think FF really does not like to switch the body.overflow properties tens time per second, the method should be executed only once at plugin init and the result cached in a variable also I don't like the way getScrollbarWidth works : if the user changed the overflow property programatically, he will loose its change

here is how I see the input with FF 26 : http://img546.imageshack.us/img546/9453/jccn.png

Also your patch about misalignment did not fix the problem in IE (tested with IE 9)

[rinogo]

Ahh, yes, I see the problem with the way getScrollbarWidth() is incorporated. Perhaps I’m hacking together this fix a bit too quickly. :) The scrollbar width is now calculated in the plugin’s init. Thanks for noticing that.

Also, I’ve updated getScrollbarWidth() to use Ben Alman’s implementation (http://benalman.com/projects/jquery-misc-plugins/#scrollbarwidth)

As far as I can tell, Firefox provides an incomplete implementation of scrollLeft(). Without it, I’m not sure how to implement this functionality. The good news is that it seems to be fixed in FF 27 (https://bugzilla.mozilla.org/show_bug.cgi?id=717878). I tested this existing implementation against Firefox Nightly (29.0), and it seems to work as expected, presumably due to the fix that is included in FF 27.

While my suggested changes might not be perfect, I do believe they’re an improvement over the current version of the plugin. With the version currently deployed to strangeplanet.fr, the following issues exist:

(For all examples, refer to the “Change highlighting color” demo):

Chrome (31.0.1650.63) - Paste in: “Lorem ipsum dolor sit amet, consectetur adipiscing elit. Quisque at massa non erat convallis vulputate molestie nec dui. Donec auctor blandit nibh quis luctus. Donec tincidunt auctor consequat.Lorem ipsum dolor sit amet, consectetur adipiscing elit. Quisque at massa non erat convallis vulputate molestie nec dui. Donec auctor blandit nibh quis luctus. Donec tincidunt auctor consequat.”

Firefox (26) and Safari (7.0.1) - Load without changes

FF:

Safari:

IE - Lots of issues.

These issues (with the exception of the IE issues) are fixed with the changes I’ve submitted. Our app is an internal-only app, so IE compatibility isn’t critical to us. Perhaps someone else who values IE compatibility can work on resolving the IE bugs.

On Dec 20, 2013, at 3:20 PM, mistic100 notifications@github.com wrote:

These issues only occur with FF, no problem with Chrome or IE I think FF really does not like to switch the body.overflow properties tens time per second, the method should be executed only once at plugin init and the result cached in a variable also I don't like the way getScrollbarWidth works : if the user changed the overflow property programatically, he will loose its change

here is how I see the input with FF 26 : http://img546.imageshack.us/img546/9453/jccn.png

Also your patch about misalignment did not fix the problem in IE (tested with IE 9)

— Reply to this email directly or view it on GitHub.

[rinogo]

Looks like the screenshots of the existing issues didn't come through from my email. Here they are:

Chrome

FF:

Safari:

[rinogo]

Hi, @mistic100! Any more thoughts on the pull request? I understand that my changes don't fix all outstanding bugs, but accepting the changes would certainly help encourage other devs to jump in and fix other bugs. I'd hate to see the whole project falter when fixes are available for at least some of the bugs! :)

[vinayvinay]

@mistic100 - can we merge this PR ? it looks good to me, especially the scrollbarWidth not being fixed at 18px. this breaks when we're using a trackpad because there's no scrollbar shown, but the JS imagines a scrollbar of width 18px.

thanks @rinogo !

[mistic100]

I trust you :) I don't want to really involve in the plugin now and take time to test

[rinogo]

You're welcome, @vinayvinay! Hope the changes work well for you. Looking forward to seeing any PR's you submit! :)

[vinayvinay]

@rinogo - did you get a chance to look at IE compatibility ? i've seen it break on IE8 for sure, where the text overlaps. do you have a work-around for that ?

[rinogo]

@vinayvinay - Nothing comes to mind at the moment; I think I do recall that there were some IE incompatibilities, but since we’re using this on an intranet, it wasn’t a concern for me. Also, I know this doesn’t really help with your problem, but IE8 is currently only at 3% of the overall global audience (http://www.w3schools.com/browsers/browsers_explorer.asp), and depending on your industry-specific audience, it could be much lower (or higher, I suppose).

You may also check for comments in the code about incompatibilities - If I found anything, that’s where I would have documented it. Best of luck! :)

[rinogo]

Also, if you can't find anything in the comments, please post a screenshot of what you're seeing, and I may be able to point you in the right direction!

[vinayvinay]

@rinogo - thanks for the inputs ! the issue i saw was overlapping text in IE8 and below. the underlying issue is with the text copied out of the text area where white-spaces and new line characters are not preserved. as a result, in the highlighting div the text comes up without newlines and white-spaces causing overlaps.

one fix was to replace \n characters with a <br />, which would take care of newline characters. yet, it is impossible to fix consecutive spaces merged into one space when text gets copied over from text area to the div. that's because there's no way to find where to backfill the consecutive spaces which got lost in translation. white-space: pre-wrap also didn't help when it came to preserving consecutive spaces.

in the end, we decided to support our feature only in IE9+ and other modern browsers.

[rinogo]

Very strange, but great description of the problem! Maybe someone else will find a workaround. Regardless, I'm glad we can both resolve this issue by simply kissing IE8 goodbye! :)

[Yaffle]

Hello, I am not using jquery-highlighttextarea yet, but I found than in Chrome input.scrollLeft returns wrong value, when brower's zoom level is not 100%, I have to multiply scrollLeft by window.devicePixelRatio to fix it. This is true ONLY for <input> element, I checked with jquery-highlighttextarea and found, that it is also broken. Did you see this issue? Do you know the bug number at https://code.google.com/p/chromium/issues/list ?