Last modified: 2011-06-28 16:12:04 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 13163 - Activate SubPageList3 extension on English Wikiversity
Activate SubPageList3 extension on English Wikiversity
Status: RESOLVED FIXED
Product: Wikimedia
Classification: Unclassified
Extension setup (Other open bugs)
unspecified
All All
: Low enhancement with 2 votes (vote)
: ---
Assigned To: Nobody - You can work on this!
http://www.mediawiki.org/wiki/Extensi...
: patch-reviewed
: 13066 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-26 14:03 UTC by James McCormack
Modified: 2011-06-28 16:12 UTC (History)
10 users (show)

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


Attachments

Description James McCormack 2008-02-26 14:03:58 UTC
Following IRC discussion and code updates, this is now on SVN. Please activate for English and Greek Wikiversity (possibly the others too - I think they all want it). Thanks.
Comment 1 Guillaume Paumier 2008-02-26 15:06:59 UTC
I guess a link to some kind of poll among local communities is needed.
Comment 3 James McCormack 2008-02-28 10:33:18 UTC
This should probably be restricted to the English and Greek Wikiversities, who produced good and unanimous votes. Further discussion has indicated that interest is not very high at the other Wikiversities at the current time.
Comment 4 James McCormack 2008-02-28 14:19:04 UTC
As regards code review, I *think* it's passed, but not quite sure. A number of devs took "a look" at it (including Werdna, Tim Starling, Splarka), of whom Werdna did the most thorough job, Tim made a quick requirement and Splarka discussed the purpose and intent. I'm not sure if this amounts to what would be called "code-review", but it certainly resulted in some changes, and Werdna uploaded it to SVN when he was happy with it. Is this what you call code-review? -- McCormack

Comment 5 Brion Vibber 2008-02-28 17:19:42 UTC
Don't turn it on unless/until I've okayed it.
Comment 6 James McCormack 2008-03-04 07:35:25 UTC
Incidentally, when this was uploaded to SVN, somebody seems to have improved my code and introduced a small bug while doing so. 
Their line: $nsi = $this->title->getNamespace();
Should be: $nsi = $this->ptitle->getNamespace();
(introduce a "p" to fix the bug)
See report by PaulHat at http://www.mediawiki.org/wiki/Extension_talk:SubPageList3 and my response to this.

Comment 7 Siebrand Mazeland 2008-03-04 07:41:43 UTC
#6 fixed in r31535
Comment 8 James McCormack 2008-03-29 05:19:14 UTC
The following comment was added by Brion Vibber to Bug ID 13066 but applies here.
---------------------------------------------------------------------------------

Made some minor code tweaks in r32559.

Some fundamental issues which need to be fixed before any Wikimedia deployment:


Sorting:

The code causes an inefficient filesort operation by sorting on
UPPER(page_title) instead of page_title. Depending on how smart the database
is, that may be a non-issue (small result data sets) or it may be wildly
inefficient, but it's a red flag.

More generally, note that the case-insensitive intention of this sort will fail
on non-ASCII characters in MySQL 4 and default compat configurations, as the
transformation applied at the DB won't be correct.

Recommend sorting simply on page_title, as elsewhere in the system (eg
Special:Prefixindex).


Caching:

Currently the extension outright disables caching on any page using the
<splist/> tag. While this ensures that up-to-date page lists are displayed, it
could make a site widely using it very expensive indeed -- the entire page,
however large, needs to be reparsed on every view -- even under a load spike
like a Slashdotting, where the same page is hit over and over.

To correctly handle cache updates, uses on a page should be registered at
links-update time, and creation/deletion of a subpage can trigger update jobs
for registered pages.
Comment 9 James McCormack 2008-03-29 05:22:17 UTC
*** Bug 13066 has been marked as a duplicate of this bug. ***
Comment 10 James McCormack 2008-03-30 12:23:27 UTC
In version 1.05 both of Brion's recommendations have been addressed.

Sorting: the "upper" has been removed.

Caching: the cache disabling in the constructor (which TimStarling and Werdna had recommended adding) has been removed so that the extension content is no longer dynamic. I discussed Brion's comment here at length with Duesentrieb, Dantman and Darkcode, all of whom see a "render dependency table" as the best way forward - but this is probably a major step which would have to wait for a future version. Even Duesentrieb's functionally related CategoryTree extension is not dynamic, so I think we can probably go initially for a simple cache-friendly user-not-quite-so-friendly version.

An SVN commit request has been made (separate bug).

Activation is requested.
Comment 11 James McCormack 2008-04-27 05:46:07 UTC
The Wikiversity voting for activation has moved around because this request is now aging. The votes are currently at: http://en.wikiversity.org/wiki/Wikiversity:Colloquium#Discussion_and_voting

Reminder: both English and Greek Wikiversities have unanimously requested activation.
Comment 12 James McCormack 2008-04-29 17:47:23 UTC
I discussed the removal of cache disabling with Brion on IRC at the beginning of April - i.e. taking a provisional simple-safe method on the caching issue. Brion asked questions and then agreed with this route. Activation was requested.

As of 29th April, the situation seems to be that the extension is now queued for "full review". Please review and activate. Thank you.
Comment 13 Cary Bass 2008-06-26 21:07:42 UTC
I've had a discussion on IRC with some of the community members and they do seem quite anxious for the bug to be implemented.  I guess it's just a matter of Brion's okaying it. 
Comment 14 Brion Vibber 2009-02-26 00:16:39 UTC
Assigning this to myself for review so it's not forgotten forever. :)
Comment 15 Tim Starling 2011-03-25 01:40:17 UTC
There are some style issues which anyone could fix up, like using English for variable names:

$parlv = substr_count($this->ptitle->getPrefixedText(), '/');

whitespace, lack of line breaks where there should be line breaks:

switch( strtolower( $options['showpath'] ) ) {
	case 'no': $this->showpath = 'no'; break;
...
if( $this->mode != 'bar' ) $pn = $this->token . $pn;

Also, it uses non-portable, unnecessary SQL identifier quoting:

$options['ORDER BY'] = '`page_title`' . $order;
...
$conditions[] = '`page_title` ' . $dbr->buildLike( $parent . '/', $dbr->anyString() );

It uses Parser::parse() when it should be using Parser::recursiveTagParse(). This is an efficiency issue, since parse() does link existence checks immediately, whereas recursiveTagParse() defers them until the next batch. It's also a correctness issue, since recursiveTagParse() guards certain syntax elements against reinterpretation, whereas parse() does not. 

Otherwise it should be OK to enable.
Comment 16 Sam Reed (reedy) 2011-04-07 00:02:22 UTC
r85590, r85593, r85594 does all the style cleanup

r85596 does the parser issue


Tim, if this is clean now, can it be pushed out?
Comment 17 Tim Starling 2011-06-28 06:52:04 UTC
Done. It's enabled on all Wikiversities, not just English.
Comment 18 Jeroen De Dauw 2011-06-28 11:40:30 UTC
I actually forked the extension and rewrite most of it at the beginning of January. See https://secure.wikimedia.org/wikipedia/mediawiki/wiki/Extension:SubPageList

This extension fixes a number of bugs in SubPageList3, cleans up the code, addresses some layout issues and is actually maintained. I (obviously) suggest using this one instead.
Comment 19 Sam Reed (reedy) 2011-06-28 15:22:22 UTC
(In reply to comment #18)
> I actually forked the extension and rewrite most of it at the beginning of
> January. See
> https://secure.wikimedia.org/wikipedia/mediawiki/wiki/Extension:SubPageList
> 
> This extension fixes a number of bugs in SubPageList3, cleans up the code,
> addresses some layout issues and is actually maintained. I (obviously) suggest
> using this one instead.

Validator has to be reviewed aswell to be able to do that
Comment 20 Jeroen De Dauw 2011-06-28 15:38:28 UTC
Ah, I suspected this was the issue. Fair enough - as long as you know there is a "newer version".
Comment 21 Sam Reed (reedy) 2011-06-28 16:12:04 UTC
(In reply to comment #20)
> Ah, I suspected this was the issue. Fair enough - as long as you know there is
> a "newer version".

Not really, AFAIK it wasn't even considered to be installed as the request was for Subpages3, and no one commented before...

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


Navigation
Links