Catrope has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/343433 )
Change subject: Watchlist: Fix form and preference overriding
......................................................................
Watchlist: Fix form and preference overriding
Fix a regression caused by aa063f4c5a19. Restore the behavior
where:
* Without action=submit: Boolean preferences apply to the displayed form and its
results, but they can be overriden by the query string.
* With action=submit: Only boolean parameters in the query string apply;
preferences are not considered.
(However, boolean preferences not on the form, i.e. 'extended', still
always apply.)
This behavior is a consequence of how checkboxes work in HTML; only
checked boxes are sent to the server.
Bug: T160734
Change-Id: Ic050dd1445ade9449496bb051f04ca0a87b8b5d7
(cherry picked from commit bc18aa06773bbb874540569aef7f530ad51013bf)
---
M includes/specials/SpecialWatchlist.php
M tests/phpunit/includes/specials/SpecialWatchlistTest.php
2 files changed, 175 insertions(+), 4 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/33/343433/1
diff --git a/includes/specials/SpecialWatchlist.php
b/includes/specials/SpecialWatchlist.php
index 5d7fa5d..4a17533 100644
--- a/includes/specials/SpecialWatchlist.php
+++ b/includes/specials/SpecialWatchlist.php
@@ -165,10 +165,6 @@
$opts->add( 'days', $user->getOption( 'watchlistdays' ),
FormOptions::FLOAT );
$opts->add( 'extended', $user->getBoolOption( 'extendwatchlist'
) );
- if ( $this->getRequest()->getVal( 'action' ) == 'submit' ) {
- // The user has submitted the form, so we dont need the
default values
- return $opts;
- }
return $opts;
}
@@ -214,6 +210,26 @@
}
}
+ if ( $this->getRequest()->getVal( 'action' ) == 'submit' ) {
+ $allBooleansFalse = [];
+
+ // If the user submitted the form, start with a
baseline of "all
+ // booleans are false", then change the ones they
checked. This
+ // means we ignore the defaults.
+
+ // This is how we handle the fact that HTML forms don't
submit
+ // unchecked boxes.
+ foreach ( $this->filterGroups as $filterGroup ) {
+ if ( $filterGroup instanceof
ChangesListBooleanFilterGroup ) {
+ foreach ( $filterGroup->getFilters() as
$filter ) {
+
$allBooleansFalse[$filter->getName()] = false;
+ }
+ }
+ }
+
+ $params = $params + $allBooleansFalse;
+ }
+
// Not the prettiest way to achieve this… FormOptions
internally depends on data sanitization
// methods defined on WebRequest and removing this dependency
would cause some code duplication.
$request = new DerivativeRequest( $this->getRequest(), $params
);
diff --git a/tests/phpunit/includes/specials/SpecialWatchlistTest.php
b/tests/phpunit/includes/specials/SpecialWatchlistTest.php
index 6e702b6..66d35a5 100644
--- a/tests/phpunit/includes/specials/SpecialWatchlistTest.php
+++ b/tests/phpunit/includes/specials/SpecialWatchlistTest.php
@@ -8,6 +8,41 @@
* @covers SpecialWatchlist
*/
class SpecialWatchlistTest extends SpecialPageTestBase {
+ public function setUp() {
+ parent::setUp();
+
+ $this->setTemporaryHook(
+ 'ChangesListSpecialPageFilters',
+ null
+ );
+
+ $this->setTemporaryHook(
+ 'SpecialWatchlistQuery',
+ null
+ );
+
+ $this->setTemporaryHook(
+ 'ChangesListSpecialPageQuery',
+ null
+ );
+
+ $this->setMwGlobals(
+ 'wgDefaultUserOptions',
+ [
+ 'extendwatchlist' => 1,
+ 'watchlistdays' => 3.0,
+ 'watchlisthideanons' => 0,
+ 'watchlisthidebots' => 0,
+ 'watchlisthideliu' => 0,
+ 'watchlisthideminor' => 0,
+ 'watchlisthideown' => 0,
+ 'watchlisthidepatrolled' => 0,
+ 'watchlisthidecategorization' => 1,
+ 'watchlistreloadautomatically' => 0,
+ ]
+ );
+
+ }
/**
* Returns a new instance of the special page under test.
@@ -29,4 +64,124 @@
$this->assertContains( '(nowatchlist)', $html );
}
+ /**
+ * @dataProvider provideFetchOptionsFromRequest
+ */
+ public function testFetchOptionsFromRequest( $expectedValues,
$preferences, $inputParams ) {
+ $page = TestingAccessWrapper::newFromObject(
+ $this->newSpecialPage()
+ );
+
+ $context = new DerivativeContext( $page->getContext() );
+
+ $fauxRequest = new FauxRequest( $inputParams, /* $wasPosted= */
false );
+ $user = $this->getTestUser()->getUser();
+
+ foreach ( $preferences as $key => $value ) {
+ $user->setOption( $key, $value );
+ }
+
+ $context->setRequest( $fauxRequest );
+ $context->setUser( $user );
+ $page->setContext( $context );
+
+ $page->registerFilters();
+ $formOptions = $page->getDefaultOptions();
+ $page->fetchOptionsFromRequest( $formOptions );
+
+ $this->assertArrayEquals(
+ $expectedValues,
+ $formOptions->getAllValues(),
+ /* $ordered= */ false,
+ /* $named= */ true
+ );
+ }
+
+ public function provideFetchOptionsFromRequest() {
+ // $defaults and $allFalse are just to make the expected values
below
+ // shorter by hiding the background.
+
+ $page = TestingAccessWrapper::newFromObject(
+ $this->newSpecialPage()
+ );
+
+ $this->setTemporaryHook(
+ 'ChangesListSpecialPageFilters',
+ null
+ );
+
+ $page->registerFilters();
+
+ // Does not consider $preferences, just wiki's defaults
+ $wikiDefaults = $page->getDefaultOptions()->getAllValues();
+
+ $allFalse = $wikiDefaults;
+
+ foreach ( $allFalse as $key => &$value ) {
+ if ( $value === true ) {
+ $value = false;
+ }
+ }
+
+ // This is not exposed on the form (only in preferences) so it
+ // respects the preference.
+ $allFalse['extended'] = true;
+
+ return [
+ [
+ [
+ 'hideminor' => true,
+ ] + $wikiDefaults,
+ [],
+ [
+ 'hideMinor' => 1,
+ ],
+ ],
+
+ [
+ [
+ // First two same as prefs
+ 'hideminor' => true,
+ 'hidebots' => false,
+
+ // Second two overriden
+ 'hideanons' => false,
+ 'hideliu' => true,
+ ] + $wikiDefaults,
+ [
+ 'watchlisthideminor' => 1,
+ 'watchlisthidebots' => 0,
+
+ 'watchlisthideanons' => 1,
+ 'watchlisthideliu' => 0,
+ ],
+ [
+ 'hideanons' => 0,
+ 'hideliu' => 1,
+ ],
+ ],
+
+ // Defaults/preferences for form elements are entirely
ignored for
+ // action=submit and omitted elements become false
+ [
+ [
+ 'hideminor' => false,
+ 'hidebots' => true,
+ 'hideanons' => false,
+ 'hideliu' => true,
+ ] + $allFalse,
+ [
+ 'watchlisthideminor' => 0,
+ 'watchlisthidebots' => 1,
+ 'watchlisthideanons' => 1,
+ 'watchlisthideliu' => 0,
+ ],
+ [
+ 'hidebots' => 1,
+ 'hideliu' => 1,
+ 'action' => 'submit',
+ ],
+ ],
+ ];
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/343433
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic050dd1445ade9449496bb051f04ca0a87b8b5d7
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: wmf/1.29.0-wmf.16
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Mattflaschen <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
