Page MenuHomePhabricator

jquery.byteLimit.js should use the length of result instead of current value + typed character
Closed, ResolvedPublic

Description

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


Version: unspecified
Severity: normal

Details

Reference
bz29467

Event Timeline

bzimport raised the priority of this task from to High.Nov 21 2014, 11:25 PM
bzimport set Reference to bz29467.

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

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.

Also need to take into account that text may come from paste operations rather than keypresses.

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 ?

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.

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

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.

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.

(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).

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)?

(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..

jquery.bytelimit was rewritten, was this adressed by the rewrite?

Indeed.

Change-Id: I9c204243d0bdf7158aa86a62b591ce717a36fe27