This action have ALWAYS had this problem: when creating a new filter,
the temporary ID is 'new', and the throttle ID is then 'new'.
This is used when creating/checking throttle keys to determine if the
user should be throttled. However, the 'new' key is not unique and
(although it's not the only part of the key), it may lead to
unpredictable behaviours. I'm not sure if this solves the task below,
but can probably help.
Also added a FIXME that we should handle shortly.
Bug: T195699
Change-Id: Id3b0ff524c52fb57fdd72f9608f758f8383e4320
Again, we're left with only one exclusion that I don't know how to fix.
See phab for a longer explanation.
Bug: T178007
Change-Id: I017097abef755bc65c77a5658ad92320bc42d78b
This test checks every deprecated variable to be identical to the
newly-named one, and to emit a debug notice. It also changes such debug
to be emitted via logger instead of wfDebug.
Bug: T201193
Bug: T173889
Change-Id: Ie55746bb7731062ae2d46d84857af2a05d78cf4c
Avoid using preg_match's offset since it is MB-unsafe. Also, remove 'UTF-8'
from mb_ functions (it's the default), reduce code duplication, and show
the right snippet for long search patterns.
Bug: T202310
Change-Id: Ieb06bdd80b0f915609afed7c7ad95e6318058ee9
As we do for user_age, since these will always change. Also, rework the
method to avoid repetition of unset().
Change-Id: Ie5ceedd89cae3813bacf6680d588bc925362c2c2
This wasn't changed in I5c370b54e6516889624088e27928ad3a1f48a821 but
really needs to be merged, to avoid setting wrong variables. At the
moment this is still fine due to temporary overrides in
AbuseFilter::generateTitleVars, but this should be merged ASAP anyway.
Bug: T173889
Change-Id: I2e6058a6fa122470a30cd4a96c68eccc66e18ae4
Align the checkbox label on the left to conform with dropdowns, avoid
two if with the same conditions, and give variables a better name. Also,
remove an unused message: with OOUI, the old design can't be reproduced.
We could add a fieldset, but then it would be greatly different from
options for other actions.
Change-Id: Ibdc993c1457636215601eb22f5202d2f6ad57bd9
The condition always evaluated to true: for global filters strpos
returned 0, otherwise it returned false, which is == 0. Fortunately, in
the second case the function returned false as it should. Anyway, be
safe and use === operator as it should always be for strpos.
Change-Id: I7ffc990b2b8b9c47ebfb64d5234f561faaff5e88
This will make tokenizer almost fully covered. The only uncovered parts
are the one with cache and an else condition which I think won't ever be
executed, and thus added a comment for that. Also, remove an obsolete
xxx comment from ComputedVariable (fixed in
I8e420f0259ef6c9e579f7a00beb58f28af9da37d)
Bug: T201193
Change-Id: I6e9a73aa9e437f096f6a1e20d53a7cb50e5ed85d
AbuseFilterParser::setVarsimply calls the setVar method in
VariableHolder and is currently unused. Its only call was removed in
I80cbc4033ff96f2fe8c1da263b1877bfb4c7c0c4. After this patch we'll only
have an uncovered line in the parser, which is likely due to a bug in
the coverage check.
Change-Id: Ic860b03b2d23fec073a9294e356e074ae1b14ae5
Several improvements, this is the list:
*Added highlighting for disabled and deprecated variables
*Simplified a bit Ace's keyword mapper
*Added highlighting for ternary operator
*Added logic to retrieve operators from AF tokenizer
*Removed $ symbol since it's not usable in declaring stuff
*Customized highlighting via CSS
Depends-On: I5c370b54e6516889624088e27928ad3a1f48a821
Change-Id: If95e34fc7260413c4fb39c18a1ef44f5a93e1a68
Adds page_age variable that reports the number of seconds since the
first edit to the current article (or 0 for new articles).
Bug: T30844
Change-Id: I0993cecc322806382a1b567b60c0a4af69054841
There is already one for the js field, but we can't reuse it since "one
by one" doesn't make any sense here.
Change-Id: Iaf01e19f4006b3d578bb2201cf9108fe46d56085
Like we do in core for similar special pages. This is really helpful
when (un)hiding an entry.
Bug: T200645
Change-Id: I16450a2573e8987e31a83ec34f3dbb16fac94f81
Variables regarding title (full list in task description) are quite
deceiving, since they use "text" instead of "title". As proposed in the
task, this is the first patch to add aliases for those variables and
slightly deprecate the old ones. In the future we may be able to replace
every occurrence (either with a search function or directly on the
database), but even a coexistence would be enough to avoid
confusion. A wfDebug log is generated whenever a deprecated variable is
parsed. The "article_" prefix is also changed to "title_", in the same
way as above.
Also, added a hook which other extension may use to specify their
deprecated variables, which will be handled the same as core ones.
Bug: T173889
Change-Id: I5c370b54e6516889624088e27928ad3a1f48a821
Add some tests and improve others to raise coverage percentage. This
should lead to almost 100% for the AbuseFilterParser class. Aside from
this, a couple of changes:
* Remove an unused function
* Let equals_to_any return a genuine result with empty strings
* Remove an if which will never be true in skipOverBraces, since the
function is called after checking the same conditions.
Bug: T201193
Change-Id: I7020b2ed996236c38c5784d161ad98ec44163406
Check if the entry is deleted first, since it's the strongest deletion
here (oversight level). Bonus: don't use implicit conversion when
checking the return value of SpecialAbuseLog::isHidden.
Bug: T200644
Change-Id: Ie5c4575ad29fe3dcb85a26cc74f1c59207df2852
Add an AbuseFilter log link to the subtitle of history pages.
Bug: T28934
Co-authored-by: Matěj Suchánek
Depends-On: I2e0e9e92d3fc303135b0eb9acf06b5fd120178a5
Depends-On: I58a3039b3755648bb0c8aaf87db48ace96ce9344
Change-Id: Ib89c48f2b8f3121ead184844844acee436e2fdd6
The following sniffs are failing and were disabled:
* MediaWiki.Commenting.FunctionAnnotations.UnrecognizedAnnotation
* MediaWiki.Usage.InArrayUsage.Found
Change-Id: I46e414246c6597dd78b069f753d686c0d1c1c09d
A month ago SpecialBlock::getSuggestedDurations has been
modified, and now it also returns an "other" key. Since we don't need it
and it would break thing up, add a parameter to avoid dealing with that.
Depends-On: Ic2dbc961f7eebad11da53724b9cce2f804ffad39
Change-Id: Ica37ba7015a04445c2cbafebcc85726368e23cb0
This settles almost everything, leaving the tags part ready to be
further improved in the follow-ups.
Also, replaced some fields with totally different ones, improved the
warn preview area and improved a bit nojs experience by hiding unusable
buttons.
Bug: T132284
Bug: T154749
Change-Id: I7a5caa862a32f9792140c6a4d9708a2d20472672
For the counting part I used this a relatively simple approach. It might
not be the best one, but should work without changing too much code. As
for the exception, I added it to every function which takes a single
parameter. Plus a couple of minor fixes: removed an unused function and
replaced "__METHOD__" with function names.
Bug: T198300
Change-Id: I484fe2994292970276150d2e417801453339e540
This line first used to be just an "strlen". Then we merged
Iaeae672dca66ffc745054daabd6f0eae7dfbc648 to clean input and this caused
some "undefined index" notices. These were in turn fixed in Ibebedb566da705e77ffb831ebda6476adba07c93 by adding an "empty". However, this slightly changed the range of accepted parameters, for instance refusing 0 and '0'. Those should never be used, so this is just a theoretical problem, but we'd better be consistent and simplify this line.
Change-Id: I4643d0632acf5926ac8de5da9bcb3e5dc715fdc1
Otherwise, if the user is warned for e.g. trying to move a page, and
after the warning he tries to delete it, he won't be warned again. Since
filtered action (edit, move, delete...) can be really different, we
should repeat the warning if the action changed.
Bug: T199621
Change-Id: Ia481b2bf552e16de8485c246aa5612d5bb2cd6ca
The patch adds the logid parameter to the queryAbuseLog API, so that
users will be able to retrieve a single result with the given logid.
Bug: T36731
Change-Id: I9160c3690e86ea40560f6fa7721918965234c29e
The function used to determine if a row is hidden has three possible
return values: true, false and "implicit". While the first and the
second one refer to AF own suppressing system, 'implicit' means that the
revision associated with the log entry is deleted. However, we checked
for such return value with a boolean cast, which caused true and
'implicit' to be equally treated, thus hiding revdel'ed revisions to
sysops. Bonus: fixed a comment typo.
Bug: T191699
Change-Id: I87d3a6437bb966198175e4bfd063e30ed79c345f
We still had three entries of "LogPage", which is legacy and has some
problems (I7bb0e92b2906a2511fc4290bdc76fc39ec4617fe). This patch updates
two of them to ManualLogEntry. The last one is handled separately in
Ic23e724997e4748c8d0da8138aa73d31b17b7064.
Change-Id: I2a4f18ea6baebdc114078c57d8937ce4ca2aace5
I've been noticing this problem for a long time: sometimes, when the
filter editor stays open for a long time and you try to click "save filter",
the page is scrolled and the edit isn't save (while it is indeed saved
when clicking save again). I found out that this is due to edit token
not matching. If that happens and the request was posted, warn user to
re-save the edit.
Change-Id: Id0c5600bf22632f57d237a19b492cc9c297be736
To generate an OOUI-friendly array with dropdown values, we need to
rearrange the array we already get from AbuseFilter::getBuilderValues().
Right now we do it in a pretty dirty way, which also causes errors if
external values (e.g. Flow variables) are in the list. With this patch,
such conversion is simplified, explained in a comment, and doesn't
output errors anymore.
Change-Id: I1063865aeff2dfb637e95d7b2ff30da39ceeab67
This is something that hasn't been working since January 2009, when AF
didn't have arrays and all variables were computed non-lazily. In fact,
when reverting "degroup", we used to take old groups from edit vars, but
the variable may not have been computed for such edit. Plus, we treated
the var collection as an array instead of an AbuseFilterVariableHolder
object, and exploded user_groups since it was a string. With this patch
everything should start working as intended.
Change-Id: I76917b2e331291bd42daeef8d048507dc38048cb
The message 'abusefilter-edit-notallowed' is used twice and outputted
as plain text. This makes it really, really hard to notice. Wrap it in a
block-level errorbox to make sure users see it.
Change-Id: I6e5579f9a5e33f05520001e10ffdde928ffdcff0
Introduced in I75ce47d247cf6949117370c8c78ab7c6980538f3, the message name
was misspelled in the code and thus the message doesn't show.
Change-Id: Iad515c48035259340c4824d456a14010c977e7a8
Html::warningBox makes use of Html::rawElement, where as noted in docblock the given html must *not* be escaped. Plus, bold text was broken due to escaping.
This reverts commit 7dfe4bfcfd.
Change-Id: I505be036291d4c6ff33c0c4fed4dd83a5bb56c54
This fixes the following minor issues:
* In HistoryPager's getQueryInfo, afh_id was listed twice
* In AbuseFilter::translateFromHistory a field named "af_" was produced
if no actions were in use
* The topnav link "Recent filter changes" wasn't STRONGed on pages like
"Special:AbuseFilter/history/123"
* In checkAllFilters and AbuseFilter::getFilter, select from DB only the
fields that will be used.
* Simplify some inline comments and remove superfluous ones
Change-Id: If72b18bedac5e580487406e696aea1fd172ae45b
Right now it can manually be added when creating filters. Since the
distinction is interal to AbuseFilter, we can't use hooks to achieve the
goal (the tag isn't already usable from outside AF). Also making
isAllowedTag public to make it testable.
Change-Id: I75ce47d247cf6949117370c8c78ab7c6980538f3
Trying to write unit tests, there are some things in the code that make
it not well testable. Here, two of them are corrected:
1 - Use class constants instead of static variables inside a non-static
method. Otherwise such variables won't be reset between tests. The
change is made so that there'll be less impact on blame.
2 - Set af_enabled to true even in af_deleted is true as well. For three
reasons: the first is that we already perform validation for this, so no
need to secretly change the option to whatever we think would make
sense. Second, this redundant validation makes some tests fail. Third:
this way, if the user selects both enabled and deleted, when the warning
is shown he'll indeed see that both checkboxes are selected. Before, he
would only see wpFilterEnabled as selected.
Change-Id: Ib7a0335fa7fb3b8a21765438a720205656c1ea09
Actually, it seems like I almost got it right at the first try. I tested
every validation scenario and it worked as espected, so ready for
review.
Bug: T193596
Change-Id: I7fd1798030d83292ce46543e25c0c431ec345a28
All uses of "throw" inside AbuseFilterParser are now covered.
Bonus: added a standard suppresswarning when checking regex validity.
Change-Id: Iacb8f7a361079e3e117dc6845597c7bd8473e54a
doLevel- functions are currently documented in AFPToken. This patch
copies such comments on docblocks in AbuseFilterParser, the place where
this docs can really be helpful.
Change-Id: I4e47e760a56800faa9b0a1146e0d79f8955dca9a
Otherwise, a blank page will be considered as having a newline inside,
which won't be marked as added (or removed) in the diff. This requires
introducing a new method and leaving the old one for backward
compatibility, and may cause regressions.
Bug: T74329
Change-Id: I9a2397fd849544b499cad97a383e5331471e9d73
