Page MenuHomePhabricator

Special:Log form should be using GET not POST
Closed, ResolvedPublic

Description

From an email from @matmarex

But I just noticed that we missed another thing. The form used to use the GET method, so you could copy an URL like https://en.wikipedia.org/w/index.php?title=Special%3ALog&type=block&year=2015&month=6 afterwards; but now it is using POST. :(

Event Timeline

Prtksxna triaged this task as Medium priority.Jul 18 2018, 5:06 AM
Prtksxna created this task.

Change 446524 had a related patch set uploaded (by Prtksxna; owner: Prtksxna):
[mediawiki/core@master] LogEventsList: Use GET in HTMLForm

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

The old URLs wont work any more since we are now using wpdate instead of year or month. We could update SpecialLog#execute to check for the old parameters and use it set the FormOptions. Is this backward-compatibility needed?

matmarex added a comment.EditedJul 18 2018, 5:31 PM

The old URLs wont work any more since we are now using wpdate instead of year or month. We could update SpecialLog#execute to check for the old parameters and use it set the FormOptions. Is this backward-compatibility needed?

It might not be needed, but I think it would be nice. I know we did this at least one in the past, when we renamed some URL parameters of Special:Watchlist for consistency with Special:RecentChanges. (That code lives in SpecialWatchlist::fetchOptionsFromRequest().)

We could also map hide_xxx_log=0 to wpfilters[]=xxx?

I didn't see a test-only feature flag, so tagging as blocking train given https://test.wikipedia.org/w/index.php?title=Special%3ALog&type=block&year=2015&month=6 now no longer works as expected, whereas the previous version does, at https://test2.wikipedia.org/w/index.php?title=Special%3ALog&type=block&year=2015&month=6.

I recognise this might not be an easy fix. If that's the case, we may want to revert for now and iterate a bit more.

Change 446524 merged by jenkins-bot:
[mediawiki/core@master] LogEventsList: Use GET in HTMLForm

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

Change 446635 had a related patch set uploaded (by Bartosz Dziewoński; owner: Prtksxna):
[mediawiki/core@wmf/1.32.0-wmf.13] LogEventsList: Use GET in HTMLForm

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

Actually, URLs using year and month will work correctly (the filter is applied), because internally, we still convert wpdate to year and month (and day), since that is what the backend expects. The only bug is that the "From date (and earlier)" form in the field will not be filled in from them.

URLs using hide_xxx_log=0 will not work though. We should fix that, but I think it's okay to roll out wmf.13 before we have a fix, this seems to be a rarely used feature.

Change 446635 merged by jenkins-bot:
[mediawiki/core@wmf/1.32.0-wmf.13] LogEventsList: Use GET in HTMLForm

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

Mentioned in SAL (#wikimedia-operations) [2018-07-18T23:35:37Z] <thcipriani@deploy1001> Synchronized php-1.32.0-wmf.13/includes/logging/LogEventsList.php: SWAT: [[gerrit:446635|LogEventsList: Use GET in HTMLForm]] T199856 (duration: 00m 54s)

Change 446719 had a related patch set uploaded (by Prtksxna; owner: Prtksxna):
[mediawiki/core@master] LogPager: Add backwards-compatibility for hide_[type]_log URL params

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

There is a non-intuitive behavior in the old Special:Log, where when you select a year and month and submit the form, it bumps up the month in the form, and if you re-submit the form without making any changes yourself it keeps moving you a month ahead. For example, open:

https://en.wikipedia.org/w/index.php?title=Special%3ALog&year=2010&month=1
.........................................................................^

Month should be January, but is February because ReverseChronologicalPager#getOffsetDate bumps up the month if no $day is present (which is always the case in the old form)

This is blocking the train from moving forward. If the issue is resolved, please resolve the task. If there is a workaround, please remove this task from T191059: 1.32.0-wmf.13 deployment blockers subtasks.

zeljkofilipin raised the priority of this task from Medium to High.Jul 19 2018, 1:13 PM

Raising the priority because this task is blocking the train.

zeljkofilipin lowered the priority of this task from High to Medium.Jul 19 2018, 3:45 PM

Setting priority back to normal since it's no longer blocking the train.

There is a non-intuitive behavior in the old Special:Log, where when you select a year and month and submit the form, it bumps up the month in the form, and if you re-submit the form without making any changes yourself it keeps moving you a month ahead. For example, open:

https://en.wikipedia.org/w/index.php?title=Special%3ALog&year=2010&month=1
.........................................................................^

Month should be January, but is February because ReverseChronologicalPager#getOffsetDate bumps up the month if no $day is present (which is always the case in the old form)

Hmm, it is not obvious whether this is a bug or a feature, but I'm going to say it's a bug. I tested with MW 1.29 and this behavior was not present there. I think we can safely remove this; I don't know when it was introduced/broken.

T171110 has tracked this, by the way.

Prtksxna updated the task description. (Show Details)Jul 23 2018, 4:52 AM

Hmm, it is not obvious whether this is a bug or a feature, but I'm going to say it's a bug. I tested with MW 1.29 and this behavior was not present there. I think we can safely remove this; I don't know when it was introduced/broken.

Introduced in rMWae2f10b786aa: Add day to date filter for ReverseChronologicalPager for T120733: Improve date range searches on Special:Contributions. One of the things it adds is getDateCond now accepts day parameter which assumes that the month should be pushed when there is no $day.

I guess a better assumption would be to keep the $month the same and set $day to 1? I hope no one is depending on this weird behavior 😷


T171110 has tracked this, by the way.

Thanks for the link @matej_suchanek

Prtksxna updated the task description. (Show Details)Jul 23 2018, 5:21 AM
Prtksxna updated the task description. (Show Details)Jul 23 2018, 6:34 AM

Change 446719 merged by jenkins-bot:
[mediawiki/core@master] LogPager: Add backwards-compatibility for hide_[type]_log URL params

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

Krinkle removed a subscriber: Krinkle.Jul 23 2018, 9:08 PM
matmarex updated the task description. (Show Details)

Change 448147 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Prefill date in form on Special:Log when calling with old parameters

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

matmarex updated the task description. (Show Details)Jul 26 2018, 8:16 PM
Prtksxna closed this task as Resolved.Jul 30 2018, 10:19 AM
Prtksxna claimed this task.
Prtksxna updated the task description. (Show Details)
Prtksxna reopened this task as Open.Jul 31 2018, 1:42 AM

Patch is not merged yet, sorry!

Change 448147 merged by jenkins-bot:
[mediawiki/core@master] Prefill date in form on Special:Log when calling with old parameters

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

matmarex closed this task as Resolved.Jul 31 2018, 4:04 PM
matmarex removed a project: Patch-For-Review.
matmarex updated the task description. (Show Details)