r83067 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r83066‎ | r83067 | r83068 >
Date:03:57, 2 March 2011
Author:bawolff
Status:ok (Comments)
Tags:
Comment:
(follow-up r80554) If person typed max amount in edit summary, backspace didn't work on some versions of opera

This still doesn't recognize all special keys on affected versions of opera as
keystrokes to ignore. I don't think this is a big issue, since backspace
is recognized (which is the only key thats really important), newer
versions of Opera should not be affected, and (based on a skim of the jQuery
site) it appears that if we ever upgrade our version of jQuery, the issue
might go away. (not 100% sure about that last part)

Originally when i was testing this code, I tested in all sorts of browsers
that it would stop people from putting more text in. I never tested that
it would allow backspaces (I now have, opera was the only affected one where
it didn't work).

Thanks to Alex Smotrov for reporting issue.
Modified paths:
  • /trunk/phase3/resources/mediawiki.action/mediawiki.action.edit.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/mediawiki.action/mediawiki.action.edit.js
——@@ -11,7 +11,13 @@
1212 // JQuery should also normalize e.which to be consistent cross-browser,
1313 // however the same check is still needed regardless of jQuery.
1414
15 - if ( e.which === 0 || e.charCode === 0 || e.ctrlKey || e.altKey || e.metaKey ) {
 15+ // Note: At the moment, for some older opera versions (~< 10.5)
 16+ // some special keys won't be recognized (aka left arrow key).
 17+ // Backspace will be, so not big issue.
 18+
 19+ if ( e.which === 0 || e.charCode === 0 || e.which === 8 ||
 20+ e.ctrlKey || e.altKey || e.metaKey )
 21+ {
1622 return true; //a special key (backspace, etc) so don't interfere.
1723 }
1824

Follow-up revisions

RevisionCommit summaryAuthorDate
r839321.17wmf1: MFT r78990, r79844, r81548, r82022, r82193, r83061, r83067, r83583,...catrope17:59, 14 March 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r80554(follow-up r66913) Per CR, make the editsummary length checker use jQuery/RL ...bawolff03:59, 19 January 2011

Comments

#Comment by Bawolff (talk | contribs)   02:15, 8 March 2011

Just wondering, why was the 1.17 tag removed from this. Its fixing something that is broken (on Opera anyways) in 1.17. Isn't that the type of thing that should be merged in?

#Comment by Catrope (talk | contribs)   10:10, 8 March 2011

If the code it's fixing were in 1.17 in the first place, yes.

#Comment by Bawolff (talk | contribs)   00:10, 9 March 2011

In that case, re-adding 1.17, 1.17wmf1 tags.

#Comment by Catrope (talk | contribs)   10:49, 9 March 2011

What I meant to say was that the code this fixes isn't in 1.17wmf1.

#Comment by Bawolff (talk | contribs)   21:46, 9 March 2011

It is in 1.17wmf1. The original issue this is fixing was reported to my talk page based on issue observed on Wikimedia sites. (I'm not really sure how the reporter found my talk page...)

#Comment by Catrope (talk | contribs)   15:51, 12 March 2011

Where is it, then? I can't find e.which anywhere in mediawiki.action.edit.js or skins/common/edit.js

#Comment by Catrope (talk | contribs)   11:37, 13 March 2011

No idea how I missed that before, thanks.

Status & tagging log