Page MenuHomePhabricator

limit=50 on RC gives count set in preferences, not 50
Closed, ResolvedPublic

Description

Author: RLUllmann

Description:
If (e.g.) I set number of changes to be displayed in RC in preferences to 470, then clicking on "50" in the RC display or explicitly setting &limit=50 in the url gives me 470.

Someone has used "=50" to mean "=default selected"?

Observed on en.wikt, confirmed by another user.


Version: unspecified
Severity: normal

Details

Reference
bz14659

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:11 PM
bzimport set Reference to bz14659.
bzimport added a subscriber: Unknown Object (MLST).

herd wrote:

Note that subpage syntax behaves as expected, eg: Special:RecentChanges/50 actually shows 50 instead of User::getDefaultOption( 'rclimit' )

I've just tested this and I'm seeing the same on commons, en.wp and cy.wp.
Setting the default to less than 50 produces an identical result.

RLUllmann wrote:

The "new helper class" FormOptions is causing the problem.

Caller (e.g. Special RC, but also others), has already set user default in $opts, wants this routine to override with specified values from the request.

This routine (fetchValuesFromRequest) should always set the array element, even if it is the "default". There is no reason for this check (it is trying to save an assignment? ;-) Code:

if ( $value !== $default && $value !== null ) {
    $this->options[$name]['value'] = $value;
}

should be changed to:

if ( $value !== null ) {
    $this->options[$name]['value'] = $value;
}

(the null check also seems odd; it means a request can't override the default with null? Seems to me case INTNULL needs that? Dunno.)

(meta comment: IMHO creating new "helpful" classes to just bury code deeper always risks this sort of thing; the code in the various callers was doing just fine. But never mind.)