Page MenuHomePhabricator
Create Task
Maniphest T156920

Added_lines and removed_lines are computed wrongly for MassMessage pages
Open, LowPublic
Actions

Description

Enwiki private filter 789 has had false positives on Wikipedia:Administrators' newsletter/Subscribe. An example filter log can be seen here.

Without discussing details of the private filter, the cause of this is that a condition searches the added_lines variable. Although the diff looks normal when viewed in the usual way (here), when you look at the filter log details, removed_lines contains all the old version with spacing, and added_lines contains all the new version without spacing. This applies to the delivery list and the header.

This effectively makes added_lines equivalent to new_wikitext.

Related Objects

Event Timeline

BethNaught created this task.Feb 1 2017, 3:49 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 1 2017, 3:49 PM
Huji renamed this task from AbuseFilter false positives on page with 'MassMessageListContent' content model due to inconsistent added_lines and removed_lines to AbuseFilter variables added_lines and removed_lines are incosistent with the diff view.Feb 1 2017, 6:08 PM
MusikAnimal mentioned this in T168736: AbuseFilter is reading variables incorrectly and tripping the wrong edits.Jun 23 2017, 6:34 PM
Daimona added a parent task: T190573: Redefine how added_lines and removed_lines are generated and handled.Mar 23 2018, 8:38 PM
Daimona subscribed.EditedMar 24 2018, 10:51 PM

@BethNaught This problem is strictly related to the page type. Actually, what kind of page is that? I'd like to make some tests, but I can't test filters on en.wiki (not enough rights), nor I know how to set up a page like that on my local wiki.

ADD: Oh, I see it's a delivery list for MM. I'll install it locally tomorrow in order to reproduce and possibly fix the problem. Probably we'll need to implement MM's own diff system, and see if we should also do the same for other extensions which use their own diffs.

Daimona triaged this task as Low priority.EditedMar 25 2018, 11:54 AM

My hunch was right. Since the page is generated by MassMessage in a peculiar way and uses its own diff system, it's quite obvious that using the vanilla one returns weird stuff. What we would need to do is determine if the page we're diffing is a MM one and, if so, implement and correctly use their own classes to generate the diff. Unfortunately I can't do that since I never dealt with MassMessage. However, given that really few pages use MM, this isn't terribly urgent and also not related to other problems affecting added_lines and arrays.

ADD: Of course, an alternative could be to add a wgAbuseFilterValidGroups for MassMessage, like we do for Flow, which would be cleaner.

Daimona renamed this task from AbuseFilter variables added_lines and removed_lines are incosistent with the diff view to Added_lines and removed_lines are computed wrongly for MassMessage pages.Mar 25 2018, 11:56 AM
Daimona removed a parent task: T190573: Redefine how added_lines and removed_lines are generated and handled.
Dreamy_Jazz subscribed.Dec 15 2022, 2:18 PM

MassMessage implements it's own diff engine that can be used. It's called MassMessageListDiffEngine.

matej_suchanek subscribed.Jan 14 2023, 10:44 AM
In T156920#8470905, @Dreamy_Jazz wrote:

MassMessage implements it's own diff engine that can be used. It's called MassMessageListDiffEngine.

Yeah, but it generates rich HTML for display. We only need plain text. Maybe the AbuseFilter-contentToString hook needs to be used.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL