Page MenuHomePhabricator

[wmf.11] "Number of edits to show in recent changes ..." form accepts invalid numbers
Closed, ResolvedPublic

Description

"Number of edits to show in recent changes..." form accepts values >5,000 and negative numbers.

Without RC new filters such invalid values will be displayed respectively as 5,000 and 0 values. With RC New filters such invalid values will be displayed in the number of changes selector.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 29 2017, 12:06 AM

I recently fixed the validation of the days parameter in the old UI, where it's now capped between 0 and 30. The new UI doesn't know this, and will happily display very large or negative numbers of days. The old UI does prevent limit from going negative, but it doesn't prevent it from being set arbitrarily high (we should set the maximum to 1000, and apply that to the preference too, like the watchlist pref) and the new UI does neither.

So we need to do the following:

  • Cap limit at 1000 in the backend
  • Cap the rclimit preference at 1000
  • In the RCFilters frontend, make the UI display the correct value when the value from the param (or pref!) runs afoul of the limits (0-30 for days and 0-1000 for limit)

Change 368955 had a related patch set uploaded (by Mooeypoo; owner: Mooeypoo):
[mediawiki/core@master] RCFilters: Normalize arbitrary values before adding them

https://gerrit.wikimedia.org/r/368955

Change 368955 merged by jenkins-bot:
[mediawiki/core@master] RCFilters: Normalize arbitrary values before adding them

https://gerrit.wikimedia.org/r/368955

There is Console error for "Number of edits to show..." above the 1,000 limit and RC page does not load

Uncaught TypeError: Cannot read property 'getName' of undefined
at MwRcfiltersDmFilterGroup.mw.rcfilters.dm.FilterGroup.getFilterRepresentation 
at MwRcfiltersDmFilterGroup.mw.rcfilters.dm.FilterGroup.initializeFilters
 at <anonymous>:76:963
    at Array.forEach (<anonymous>)
    at Object.<anonymous> (<anonymous>:76:761)
at Function.each (/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki%7Cmediawiki.legacy.wikibits&only=scripts&skin=vector&version=1he67vb:5)
    at MwRcfiltersDmFiltersViewModel.mw.rcfilters.dm.FiltersViewModel.initializeFilters (<anonymous>:76:614)
    at MwRcfiltersController.mw.rcfilters.Controller.initialize (<anonymous>:96:618)
    at HTMLDocument.init (/w/load.php?debug=false&lang=en&modules=ext.guidedTour.styles%7Cext.guidedT…ets%7Coojs-ui.styles.icons-editing-advanced&skin=vector&version=0omry0j:77)
    at mightThrow (/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki%7Cmediawiki.legacy.wikibits&only=scripts&skin=vector&version=1he67vb:49)

Change 369730 had a related patch set uploaded (by Mooeypoo; owner: Mooeypoo):
[mediawiki/core@master] RCFilters: Normalize user-generated default values

https://gerrit.wikimedia.org/r/369730

Change 369730 merged by jenkins-bot:
[mediawiki/core@master] RCFilters: Normalize user-generated default values

https://gerrit.wikimedia.org/r/369730

Re-checked in betalabs - the invalid numbers for the Number of changes selector (over 1,000 and negative numbers) won't be displayed.
However, if a negative number (or zero) is entered and saved in 'Number of edits to show...', RC page will behave weirdly:

  • RC page will not display any results - "No changes during the given period match these criteria."
  • the label will become 'Show last change' without displaying any number
  • 'Show last changes' selection will include 1 as an option (&limit=1 is a rounded result of any negative number to the nearest valid number?)

Note: without RC new filters, zero is an accepted input and will be displayed on RC page as an option.

Change 369810 had a related patch set uploaded (by Mooeypoo; owner: Mooeypoo):
[mediawiki/core@master] RCFilters: Normalize 'limit' to minimum 0, like the backend does

https://gerrit.wikimedia.org/r/369810

Fixed the minimum cap to 0 rather than 1, since the backend seems to do that for us too, and we should be consistent even if it is stupid.
(We should fix this in the backend, but that will change current behavior. @Catrope?)

Change 369810 merged by jenkins-bot:
[mediawiki/core@master] RCFilters: Normalize 'limit' to minimum 0, like the backend does

https://gerrit.wikimedia.org/r/369810

Checked the last fix - saving 'Numbers of edits to show...' = 0 , will be displayed as 'Show last 0 changes' which is the expected behavior. Negative numbers for 'Numbers of edits to show...' will be evaluated to 0.

QA Recommendation: Resolve

Fixed the minimum cap to 0 rather than 1, since the backend seems to do that for us too, and we should be consistent even if it is stupid.
(We should fix this in the backend, but that will change current behavior. @Catrope?)

I'd be OK with that.

jmatazzoni closed this task as Resolved.Aug 4 2017, 2:04 AM
Restricted Application added a project: Growth-Team. · View Herald TranscriptDec 1 2018, 1:46 PM