Last modified: 2009-09-15 19:30:44 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 T21621, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 19621 - Improve SpecialUserStats.php - namespace selector
Improve SpecialUserStats.php - namespace selector
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
UsageStatistics (Other open bugs)
unspecified
All All
: Normal enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
http://www.mediawiki.org/wiki/Extensi...
: patch
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-07-09 20:29 UTC by Al Maghi
Modified: 2009-09-15 19:30 UTC (History)
3 users (show)

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


Attachments
statistics per namespace (3.09 KB, patch)
2009-07-09 20:29 UTC, Al Maghi
Details
Patch corrected with no SQL through form fields (3.32 KB, patch)
2009-07-14 14:19 UTC, Al Maghi
Details
alternative with addition of a message to inform users (3.85 KB, patch)
2009-07-14 14:37 UTC, Al Maghi
Details
Patch extension and extension messages. Correction thks to Roan (3.87 KB, patch)
2009-07-14 15:11 UTC, Al Maghi
Details
Alternative patch: further improvement with namespaceSelector (4.92 KB, patch)
2009-07-14 19:33 UTC, Al Maghi
Details
use addQuotes instead of strencode, plus other improvements thks to Roan. (6.47 KB, patch)
2009-07-14 23:14 UTC, Al Maghi
Details
parent class in constructor (6.71 KB, patch)
2009-07-16 17:11 UTC, Al Maghi
Details
+casing correction (6.84 KB, patch)
2009-07-21 23:20 UTC, Al Maghi
Details
is that use of XML functions wanted? (6.96 KB, patch)
2009-07-22 23:05 UTC, Al Maghi
Details
further improvement with NikeRabbit (10.85 KB, patch)
2009-08-16 14:49 UTC, Al Maghi
Details
FIXME: JS as an external file with $wgJSAutoloadClasses (154.99 KB, patch)
2009-08-16 15:43 UTC, Al Maghi
Details

Description Al Maghi 2009-07-09 20:29:49 UTC
Created attachment 6315 [details]
statistics per namespace

Add the possibility to plot the statistics concerning Main namespace.
Comment 1 Al Maghi 2009-07-10 11:47:17 UTC
Add to query (line159) " AND page_is_redirect=0 " to get rid of statistics on redirects.
Comment 2 Al Maghi 2009-07-13 18:04:04 UTC
(In reply to comment #1)
> Add to query (line159) " AND page_is_redirect=0 " to get rid of statistics on
> redirects.

Better adding: " AND page_is_redirect=0" in function DisplayForm line 387:

<option value='=0 AND page_is_redirect=0' selected>". wfMsg('usagestatisticsnsmain') ."
Comment 3 Roan Kattouw 2009-07-13 18:07:03 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > Add to query (line159) " AND page_is_redirect=0 " to get rid of statistics on
> > redirects.
> 
> Better adding: " AND page_is_redirect=0" in function DisplayForm line 387:
> 
> <option value='=0 AND page_is_redirect=0' selected>".
> wfMsg('usagestatisticsnsmain') ."
> 

That shouldn't work. If it does, that's an SQL injection vulnerability.
Comment 4 Al Maghi 2009-07-14 12:31:53 UTC
(In reply to comment #3)
> That shouldn't work. If it does, that's an SQL injection vulnerability.
> 

Is it not rather a selection than an injection; indeed that query does not change the DB:

'SELECT ... FROM ... WHERE rev_page=page_id AND page_namespace=0 AND page_is_redirect=0'
Comment 5 Roan Kattouw 2009-07-14 13:08:49 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > That shouldn't work. If it does, that's an SQL injection vulnerability.
> > 
> 
> Is it not rather a selection than an injection; indeed that query does not
> change the DB:
> 
> 'SELECT ... FROM ... WHERE rev_page=page_id AND page_namespace=0 AND
> page_is_redirect=0'
> 

I don't think you understand. It should not be possible to inject SQL through form fields EVER, or someone will find a way to exploit it. To fix this, you should use "AND page_namespace " . $db->addQuotes( $ns )
Comment 6 Al Maghi 2009-07-14 14:19:17 UTC
Created attachment 6329 [details]
Patch corrected with no SQL through form fields

Correction thanks to Roan.
Comment 7 Al Maghi 2009-07-14 14:37:07 UTC
Created attachment 6330 [details]
alternative with addition of a message to inform users
Comment 8 Roan Kattouw 2009-07-14 14:43:26 UTC
(In reply to comment #7)
> Created an attachment (id=6330) [details]
> alternative with addition of a message to inform users
> 

These patches are still ugly as they pass >= or = as part of the namespace parameter. Instead, you should make the namespace parameter default to null and not set the page_namespace part of the query if it's null. If a namespace was selected, you should just pass the number, without the = sign.
Comment 9 Al Maghi 2009-07-14 15:11:37 UTC
Created attachment 6331 [details]
Patch extension and extension messages. Correction thks to Roan
Comment 10 Chad H. 2009-07-14 15:14:01 UTC
IMHO, rather than hard-coding this to be "all NS or NS-0 only", why not use the namespace selector in the XML class? It's already designed for this and handles all namespaces.
Comment 11 Al Maghi 2009-07-14 15:22:03 UTC
(In reply to comment #10)
> why not use the namespace selector in the XML class?

Because I didn't need statistics on other namespaces. 

This could be an improvement, could you submit a patch or detail how namespace-selector looks like?

To patch, we should keep in mind that we need a "all NS" value in selection.



Comment 12 Chad H. 2009-07-14 15:29:40 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > why not use the namespace selector in the XML class?
> 
> Because I didn't need statistics on other namespaces. 
> 

You may not, but while you're patching you might as well go for the better implementation and it'll be more helpful to everybody :)

> This could be an improvement, could you submit a patch or detail how
> namespace-selector looks like?
> 

Check out the Xml::namespaceSelector() method.

> To patch, we should keep in mind that we need a "all NS" value in selection.
> 

namespaceSelector() could use a generic way to do this, which it doesn't currently do. Might be worth patching that in too.
Comment 13 Al Maghi 2009-07-14 19:33:09 UTC
Created attachment 6332 [details]
Alternative patch: further improvement with namespaceSelector

(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > why not use the namespace selector in the XML class?
> > 
> > Because I didn't need statistics on other namespaces. 
> > 
> 
> You may not, but while you're patching you might as well go for the better
> implementation and it'll be more helpful to everybody :)
> 
There is a patch using nsSelector... That alternative patch can be cleaned up.
Comment 14 Al Maghi 2009-07-14 23:14:45 UTC
Created attachment 6334 [details]
use addQuotes instead of strencode, plus other improvements thks to Roan.
Comment 15 Al Maghi 2009-07-16 17:11:00 UTC
Created attachment 6345 [details]
parent class in constructor
Comment 16 Chad H. 2009-07-18 16:29:51 UTC
* JS file should be as an external file, use addScriptFile().
* Use consistent function casing (addHTML vs AddHTML)
* Use XML functions rather than building all HTML by hand
Comment 17 Al Maghi 2009-07-21 23:20:18 UTC
Created attachment 6380 [details]
+casing correction
Comment 18 Al Maghi 2009-07-22 23:05:49 UTC
Created attachment 6383 [details]
is that use of XML functions wanted?
Comment 19 Al Maghi 2009-08-16 14:49:22 UTC
Created attachment 6470 [details]
further improvement with NikeRabbit

use class=mw-label , mw-input. instead of align=left..


Further use of XML::element instead of html.


colon put inside the message.
Comment 20 Al Maghi 2009-08-16 15:43:37 UTC
Created attachment 6471 [details]
FIXME: JS as an external file with $wgJSAutoloadClasses

CalendarPopup() is working. showNavigationDropdowns() is working.

FIXME: the onClick event is *not* working. [<a onClick=CalendarPopup.select()]
Comment 21 Alexandre Emsenhuber [IAlex] 2009-09-15 19:30:44 UTC
Done in r56389.

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


Navigation
Links