Last modified: 2014-11-17 10:36:04 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T31467, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 29467 - jquery.byteLimit.js should use the length of result instead of current value + typed character
jquery.byteLimit.js should use the length of result instead of current value ...
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
JavaScript (Other open bugs)
unspecified
All All
: High normal with 1 vote (vote)
: 1.20.0 release
Assigned To: Krinkle
: javascript
Depends on: 39353
Blocks: 29804 38275
  Show dependency treegraph
 
Reported: 2011-06-18 01:27 UTC by Helder
Modified: 2014-11-17 10:36 UTC (History)
7 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Helder 2011-06-18 01:27:37 UTC
Currently, if a user fill the edit summary with a text which sums up 250 characters, such as
----
abcdefghijABCDEFGHIJabcdefghijABCDEFGHIJabcdefghijABCDEFGHIJabcdefghijABCDEFGHIJabcdefghijABCDEFGHIJabcdefghijABCDEFGHIJabcdefghijABCDEFGHIJabcdefghijABCDEFGHIJabcdefghijABCDEFGHIJabcdefghijABCDEFGHIJabcdefghijABCDEFGHIJabcdefghijABCDEFGHIJabcdefghij
----
the JS code from jquery.byteLimit.js will not let the user to type any other characters. So far so good. Nonetheless, it should let the user to replace a substring of the current summary (e.g. the last 10 characters), but it doesn't (even if the user selects **the whole summary** and type "why not?", which as only a few characters).

IIUC, this is because the script is testing the length of this.value[1] of the current input, instead of taking the length of the selected text into account.

[1] http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/resources/jquery/jquery.byteLimit.js?annotate=86982#l49
Comment 1 Helder 2011-06-18 01:46:30 UTC
It seems that each browser has its own way to get the selected text, so this may be useful:
http://www.codetoad.com/javascript_get_selected_text.asp
Comment 2 Helder 2011-06-18 01:52:02 UTC
Or we could add a module with a jQuery plugin for that. E.g.:
https://github.com/localhost/jquery-fieldselection/blob/master/jquery-fieldselection.js
Comment 3 Roan Kattouw 2011-06-18 13:11:50 UTC
We already have stuff for this in jquery.textSelection, although it's probably not very nice.

I don't see why we can't just let the replacement happen, check whether it violated the limit after the fact, then undo it? I guess that way a character could appear for a split second before disappearing, maybe.
Comment 4 Brion Vibber 2011-06-18 17:59:08 UTC
Also need to take into account that text may come from paste operations rather than keypresses.
Comment 5 Krinkle 2011-06-21 17:58:56 UTC
Filtering text selection is bad idea imho. Replacing selected text is just one of the menu ways of inputting text. There's only drag-and-drop and copy/paste.

Perhaps use another onkey* event instead of keypress. keyup perhaps ?
Comment 6 Brion Vibber 2011-07-11 19:19:52 UTC
Additional complications testing on Firefox 5/Ubuntu 11.04:

* using Japanese input method, no keypress events fire *at all*
* using English keyboard + compose key, composed characters (eg 'compose+apostrophe', 'e' to create 'é') do not fire keypress events

In both cases, nothing stops the text from being entered until you hit the actual maximum character count (which is enforced by the browser's widget sets)

meanwhile:
* using Japanese kana, Israeli Hebrew, or Esperanto keyboards, keypress events do fire for keys that can be typed directly

I'm still not sure what happens on the off chance that some keyboard configuration actually lets you type non-BOM characters directly -- does it give you one keypress event with the full Unicode code point number, or two (one with each surrogate character), or none like the fancy input methods?


To deal with input methods, pastes etc, we probably need to actually do the 'let it happen, then check if it was legit' method since I'm not sure we can cleanly get the change that's about to happen before it happens.


Honestly my inclination in the long term is to deprecate these byte-limited fields entirely:

* replace most random varchar(255)s with saner or unlimited in-db fields, and apply sensible char limits instead of hardcoded byte limits

* where really needed for legacy issues (eg page titles), apply the limit as part of other form field validation -- a raw byte count is insufficient in any case then as you need to normalize before checking the length. This should probably be asynchronous and not apply hard limits, but allow you to type in extra crud and just whinge at you until you fix it.
Comment 7 Brion Vibber 2011-07-11 19:25:38 UTC
Firefox 4.0 & Safari 5 on Mac OS X 10.6.8 are similar: Japanese input method actually fires a keypress for the 'return' key though on Firefox/Mac (but not Safari/Mac) -- potentially preventing the submission depending on how long text _before_ the input part was. :D
Comment 8 Krinkle 2011-07-11 19:47:32 UTC
brion and me had a little brainstorm on IRC earlier today, perhaps we can just drop trying to determine all the edgecases for insertion and handling them each. Instead don't prevent the input and evaluate input later.

So we'd bind to (some or all) of keyup, keydown, change, blur, drag*.
Then we'll catch normal types with key*, copy/paste with ctrl v with key* as well, drag* will get the dragged in text, and blur/change for paranoid sake.

On that event check the byteLength after the insertion and if too long, chop down to the limit.

That'll also remove/fix bug 29804.
Comment 9 Rainer Rillke @commons.wikimedia 2012-08-14 21:41:13 UTC
(In reply to comment #8)
>So we'd bind to (some or all) of keyup
http://stackoverflow.com/questions/6458840/on-input-change-event
Comment 10 Bawolff (Brian Wolff) 2012-08-15 12:41:39 UTC
>On that event check the byteLength after the insertion and if too long, chop
>down to the limit.

Say you have 240 bytes in the text box. You realize you forgot to write something at the beginning of your edit summary. You move your cursor to the begining of the input box and type 30 more bytes of characters. Now the byte limiter has chopped off the last 15 bytes of your summary with no indication to you (since the last 15 bytes are at end of textbox and not visible). I think this would be very confusing to the user.
Comment 11 Krinkle 2012-08-16 16:43:14 UTC
(In reply to comment #10)
> >On that event check the byteLength after the insertion and if too long, chop
> >down to the limit.
> 
> Say you have 240 bytes in the text box. You realize you forgot to write
> something at the beginning of your edit summary. You move your cursor to the
> begining of the input box and type 30 more bytes of characters. Now the byte
> limiter has chopped off the last 15 bytes of your summary with no indication to
> you (since the last 15 bytes are at end of textbox and not visible). I think
> this would be very confusing to the user.

Fair point but unless you have a better idea, that is still much better than the current way:

Enter < limit bytes of text in the box. Use any method of text insertion other than *plain* typing that will  (pasting, dragging, selecting text than typing or pasting etc.) and that method of insertion will be prevented from happening with no indication whatsoever.

Also by listening on character insertion it is unable to limit insertion through those other methods. And no, it is not feasible to try and account for all those methods because there are too many:
* typing
* selecting a word and typing more (e.g. "foo bar baz", limit 11, select "baz" type "x". this has to be impossible, but is not possible if limit is 11 and method is naive and checks length + to-be-inserted character)
* using any non-plain input (e.g. spell check correction, pasting, dragging, type suggestions on mobile devices, and custom javascript input methods for foreign characters that consider typing several characters as one (e.g. "" -> "a" -> "ae" -> "ä"). In the latter case we need to not prevent the insertion.

The best way I can see on short term is instead of listening on input, listen for changes and when they happen let the whole call stack / event loop run to completion and then chop off to the limit if needed (e.g. a 0ms setTimeout, just to put it at end of the loop to allow dom events to run, since those are synchronous).
Comment 12 Bawolff (Brian Wolff) 2012-08-16 16:51:55 UTC
>Fair point but unless you have a better idea, that is still much better than
>the current way:
>
>Enter < limit bytes of text in the box. Use any method of text insertion other
>than *plain* typing that will  (pasting, dragging, selecting text than typing
>or pasting etc.) and that method of insertion will be prevented from happening
>with no indication whatsoever.

I disagree about which one is worse. People (and by people I mean really just me, I have no idea what other people actually do) do plain typing a lot, including plain typing not at the end of the input box. They rarely input by selecting and dragging and dropping. Pasting is less rare, but I still feel that plain typing not at end of box is more common than pasting.


Can we do both (prevent input if adding the character would go over the limit, and chopping things after the fact if the user manages to somehow get something past the limit)?
Comment 13 Krinkle 2012-08-16 17:07:12 UTC
(In reply to comment #12)
> Can we do both (prevent input if adding the character would go over the limit,
> and chopping things after the fact if the user manages to somehow get something
> past the limit)?

Yes we should try, but no I have no clue how. Because our browsers have been so nice to have no way to implement this (afaik). All we can do is watch for key presses, and when they do there is no way of knowing where it came from and what it will do (e.g. Narayam[1] is also watching, and when it sees "e" after "a" it could[2] turn it into "ae". If the "a" is the last character to be insertable, we need to allow Narayam to change the value. But if we listen for keypress[3], and prevent if current value + character-of-key-being-pressed - it will not work as expected. That's exactly the problem.

I'm open to other ideas, but I think it is technically simply impossible to do both in the current browser world.

[1] https://www.mediawiki.org/wiki/Extension:Narayam
[2] I don't know whether there is a German module that does this, its just an example.
[3] Or keyup/keydown..
Comment 14 db [inactive,noenotif] 2012-11-11 11:52:32 UTC
jquery.bytelimit was rewritten, was this adressed by the rewrite?
Comment 15 Krinkle 2012-12-31 17:26:38 UTC
Indeed.

Change-Id: I9c204243d0bdf7158aa86a62b591ce717a36fe27

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links