MediaWiki r86108 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:r86107‎ | r86108 (on ViewVC)‎ | r86109 >
Date:08:23, 15 April 2011
Author:diebuche
Status:resolved (Comments)
Tags:needs-js-test 
Comment:
Followup ro r86088: Use data-sort-type instead of classes to specify the parser-type; add support for data-sort-value; strip legacy code (CR)
Modified paths:

Diff [purge]

Loading diff…

Follow-up revisions

Rev.Commit summaryAuthorDate
r86109r86108: Fix var namediebuche08:31, 15 April 2011
r86854Followup to r86108: jQuery tries to be too smart and converts the string to a...diebuche13:15, 25 April 2011
r87172Followup to r86108 per CR: Fix two calls to legacy tablesorterdiebuche18:58, 30 April 2011
r96384jquery.tablesorter.test: Add tests for data-sort-value...krinkle23:09, 6 September 2011
r96509Followup to r86108, r86854, r96384: table sorter fetch of 'data-sort-value' a...brion22:03, 7 September 2011

Past revisions this follows-up on

Rev.Commit summaryAuthorDate
r86088Completely rewritten table sorting script....diebuche21:47, 14 April 2011

Comments

#Comment by Nikerabbit (talk | contribs)   07:03, 25 April 2011

It's still not working. It always sorts by the first column regardless of what is clicked.

#Comment by DieBuche (talk | contribs)   10:57, 25 April 2011

It seems to work fine in my trunk in various browsers & tables. Could you post some details?

#Comment by Nikerabbit (talk | contribs)   12:15, 25 April 2011
#Comment by Brion VIBBER (talk | contribs)   22:30, 22 June 2011

I don't see the described problem at that URL at present; is this something that's gone away in the meantime, or do you still see it Nike?

#Comment by Nikerabbit (talk | contribs)   05:38, 23 June 2011

It was fixed in later commit.

#Comment by Krinkle (talk | contribs)   22:26, 27 April 2011

Are you sure non of these legacy global functions are used anywhere in core or by extensions ?

#Comment by DieBuche (talk | contribs)   02:52, 28 April 2011

Thanks for reminding: Legacy globals are in use at: extensions/WikiEditor/modules/jquery.wikiEditor.dialogs.config.js and copied from there to /LiquidThreads/js/lqt.toolbar.js

I'll fix those or readd the globals

#Comment by Brion VIBBER (talk | contribs)   20:07, 22 June 2011

Needs JS tests; will end up reverted along with r86088 and friends if regressions are not fixed.

#Comment by Siebrand (talk | contribs)   15:09, 6 September 2011

Just so that a reviewer does not have to look it up: r86088 was set to resolved by TheDJ on 14 Aug 2011.

#Comment by DieBuche (talk | contribs)   12:42, 6 July 2011

Remarking as new

#Comment by Krinkle (talk | contribs)   23:09, 6 September 2011

Unit tests for data-sort-value added in r96384.

#Comment by Brion VIBBER (talk | contribs)   22:14, 7 September 2011

This test uncovered a bug on IE 6/7, fixed in r96509.

#Comment by Brion VIBBER (talk | contribs)   18:07, 13 September 2011

Fixes noted above resolve known issues thx to tests -- marking resolved.

#Comment by Krinkle (talk | contribs)   23:04, 17 September 2011

Still needs a test for data-sort-type.

Status & tagging log

  • 23:04, 17 September 2011 Krinkle (talk | contribs) changed the tags for r86108 [added: needs-js-test]
  • 18:07, 13 September 2011 ^demon (talk | contribs) changed the tags for r86108 [removed: frontend,krinkle]
  • 18:07, 13 September 2011 Brion VIBBER (talk | contribs) changed the tags for r86108 [removed: needs-js-test,brion]
  • 18:07, 13 September 2011 Brion VIBBER (talk | contribs) changed the status of r86108 [removed: new added: resolved]
  • 15:52, 13 September 2011 MarkAHershberger (talk | contribs) changed the tags for r86108 [added: brion]
  • 15:10, 6 September 2011 Catrope (talk | contribs) changed the tags for r86108 [added: krinkle,frontend]
  • 12:42, 6 July 2011 DieBuche (talk | contribs) changed the status of r86108 [removed: fixme added: new]
  • 20:07, 22 June 2011 Brion VIBBER (talk | contribs) changed the tags for r86108 [added: needs-js-test]
  • 20:07, 22 June 2011 Brion VIBBER (talk | contribs) changed the status of r86108 [removed: ok added: fixme]
  • 21:57, 9 June 2011 Krinkle (talk | contribs) changed the status of r86108 [removed: new added: ok]
  • 19:00, 30 April 2011 DieBuche (talk | contribs) changed the status of r86108 [removed: fixme added: new]
  • 22:26, 27 April 2011 Krinkle (talk | contribs) changed the status of r86108 [removed: new added: fixme]
  • 22:13, 27 April 2011 DieBuche (talk | contribs) changed the status of r86108 [removed: fixme added: new]
  • 07:03, 25 April 2011 Nikerabbit (talk | contribs) changed the status of r86108 [removed: new added: fixme]