Page MenuHomePhabricator

Summaries for CheckUser.php
Closed, ResolvedPublic

Description

Author: mihai

Description:
I propuse a new field in CheckUser.php in which the checkuser should provide
small info about his/her check.

This will help the other checkuser to check the person which do the check
(according to the policy) and could be helpfull.

Original proposed by Vlad@rowiki


Version: unspecified
Severity: enhancement

Details

Reference
bz5044

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 9:08 PM
bzimport added a project: CheckUser.
bzimport set Reference to bz5044.
bzimport added a subscriber: Unknown Object (MLST).

Add reason box and a "list users only" option to checkuser

attachment CheckUser_body.php ignored as obsolete

robchur wrote:

Comment on attachment 2813
Add reason box and a "list users only" option to checkuser

That's not a patch, it's a whole new file. Please submit a *diff* against the
existing file. Your Subversion client can do this, as can diff -u on Unix/Linux
etc.

patch to add "list users only" and "reason" box to checkuser

OK, this should be in patch format now. Sorry.

attachment cu_diff.patch ignored as obsolete

robchur wrote:

I'd rather see the feature split into a new function, and presented as such - a
different submit button, perhaps, for the visual cue.

(In reply to comment #4)

I'd rather see the feature split into a new function, and presented as such - a
different submit button, perhaps, for the visual cue.

I am not sure what you mean. I was thinking of having it like the summary box
for edits, exept having a submit button for each operation (user or IP). Like at
http://www.mediawiki.org/wiki/User:Voice_of_All/CheckUser2.0. The summary is an
auxilary to either of the twain, so I don't see why it would need its own submit
button.

Can you expand on what you mean by "visual cue"? Maybe I code make some code
changes, but I really want to keep this simple.

robchur wrote:

I was on about the "usernames used" bit, since you seem to have presented two
fixes in the same patch...which is not always a good idea.

Patch to add "reason" box and "list usernames only option"

OK, I've taken Rob's advice and forked the interface (see my mediawiki.org
subpage) and the functions. Having done that, I've added a bit of info to the
"get users" results, inclusing time framing, a count of the edits, and the user
tool links.

attachment checkuser2.patch ignored as obsolete

update; cleaned up some things

attachment checkuser2.patch ignored as obsolete

upate; clean up form

attachment checkuser2.patch ignored as obsolete

Summary patch for checkuser (see http://www.mediawiki.org/wiki/User:Voice_of_All/CheckUser2.0)

attachment checkuser2.patch ignored as obsolete

Summary patch; pretty up form and fixed two small bugs

attachment checkuser2.patch ignored as obsolete

A few quick notes:

  • some spacing and capitalization quirks; not a big deal but it's nice to be

clean ;)

  • the form HTML isn't XHTML-clean. the old one wasn't either, but it's good to

try to clean that up

  • i18n appears incomplete; "<checkuserheader>" appears at top of form
  • if I just hit 'enter' in the form without inputting anything, i get a php warning:

<b>Notice</b>: Undefined variable: counter in
<b>/opt/web/pages/trunk/extensions/CheckUser/CheckUser_body.php</b> on line
<b>130</b><br />

  • it's not clear which button will be active when i hit 'enter' in the ip field
  • "Get users" mode seems to have some bugs, spews out php warnings:

<b>Notice</b>: Undefined index: WikiSysop in
<b>/opt/web/pages/trunk/extensions/CheckUser/CheckUser_body.php</b> on line
<b>162</b><br />
etc for each username that appears

  • hitting 'enter' in the username field appears to have no effect; it's

triggering the 'Get edits' for IP button, at least in Firefox

  • the 'search' box on the log view submits to an incorrect address on server

with query string-based article URLs; the target page needs to be added as a
hidden field to the form

Fixes:
*I've spaced the form out a bit better
*Removed the header (maybe later)
*The 3 query types our only ran if the corresponding input is given, so pressing
"enter" either takes you to the normal checkuser page without running any
queries, or, if there is a valid input pair (like "get users" and an IP is
given) it will run. This should fix the botched query problems.
*$Counter is always defined (rather than only when results are returned)

Todo:
*XHTML compatibility: Not really sure how to do this
*Search box address: Brion, could you explain this more, and how can I repeat
the error? I don't understand what the bug is but I'd like to fix it.

Also, I never got the "Undefined index" error on my test wiki. The output
appears fine and no php errors show (I have php set to show any error or
notice). I did change key checker to use an appropriate function,
array_key_exists(), but I don't know if that fixes the bug because I never
experienced it.

XHTML: use well-formed XML in output. A quick way to test this is to set
$wgMimeType = 'application/xhtml+xml'; If you're using Firefox/Mozilla or Opera,
this will nicely run output through a proper XML parser and spit nasty errors
at you if your code is bad. You can also do a full validity check with
validator.w3.org or something for more detailed checks.

Common errors are unclosed tags, misnested tags, and unquoted attributes (for
instance on the table tags).

Um, did you want to attach the updated patch so it could be tested? :)

The search issue:

a) The action URL on the form is the default article URL for Special:CheckUser.

b) The form submits by GET, meaning that form fields are formatted into a query
string on the target URL.

If $wgArticlePath uses the query string to specify titles, the result is that
the query string with the title is stripped from the URL and replaced with the
fields in the form. Since the form doesn't include a "title" field, the data is
submitted to the default main page instead of to Special:CheckUser.

For this sort of thing, use $wgScript as the target URL and pass the special
page name in a hidden title field, ensuring it ends up on the submission URL.

update summary patch; XHTML valid, and script title fixed

attachment checkuser2.patch ignored as obsolete

(The other fixes are included in the last patch as well)

I get no results when hitting 'enter' in the user field...
Actually, I get edits-for-ip results if the IP field is also filled out at the
time -- very confusing when trying to narrow down results or such.

There's still no indication which button will be default when I hit enter in the
IP field.

I get a new notice error when listing users for an IP:
<br />
<b>Notice</b>: Undefined variable: s in
<b>/opt/web/pages/trunk/extensions/CheckUser/CheckUser_body.php</b> on line
<b>177</b><br />

The behavior when clicking on an IP in the ips-for-user list is a bit
non-obvious to me. I guess it pre-fills the form for an IP-to-edit check?

Spacing/alignment on the form is a bit off; 'IP address' is vertically centered
between the input box and buttons, but 'Username' is aligned with the input box
alone. 'Username:' is also flush against the input box, and should probably have
some free space.

The new code doesn't insert <li>...</li> wrappers into the log file, but doesn't
strip the old ones when reading log entries. This leads to incorrect output
display, with apparently blank lines or other oddities.

Most of the other problems seem fixed, though. Good job!

Sorry, didn't want to be all negative there. ;)

Update; $s initialized, alignment stuff, User button works with enter

OK, I've switched the places of IP and User after doing some other alignment
stuff. This way enter works for User checks and you usually check names before
checking IPs. Pressing "enter" for IPs has no affect; it returns to the
standard checkuser page.

"Enter" only works with simple inputs, browsers tend to just pick one submit
tab and choose it (and they pick different ones, Opera and Mozilla). Opera at
least highlights the one it will submit.

I could have "get edits" say "(default)" and make a form for both IPs and Users
which would allow enter to work fully *where* I not also having a reason box.
The only way around that would be to have two forms and a silly reason box for
each :).

attachment checkuser2.patch ignored as obsolete

Oh, also see http://www.mediawiki.org/wiki/User:Voice_of_All/CheckUser2.0. The
<li> tags need to be removing from the file (open it and use replace all or
something?). That way it will parse correctly.

robchur wrote:

What benefit does the new parse method have over the old one, i.e. exactly what
benefit would we encounter after hacking about with a huge log file?

The search box should only search for things visible in the log. Since you don't
see formatting tags, then search shouldn't pick up them. This can be done by
having only delimeters as non-visible characters in the log, like \n. The <li>
tags are imploded in when viewed. This means that if you type in "<li>", "<", or
">", you won't get a the whole log. It also makes the file a tad smaller, but
thats not the main reason.

The enter behavior is just really weird to me. I'm also bugged more and more by
the reason box being above, when it's below in other forms.

Two separate sub-forms probably makes more sense than unpredictable behavior in
a unified form.

I'd also prefer compatibility with existing log files; there's no reason to
break compatibility.

Switch back to old log format, hacks to get enter to "work"

OK, the search now uses a hack to ignore the <li> tags, so the format is back
to the old way.

As for the reason box, its best left on top as in other forms, there is only
one "OK" box right after the reason box. In this case, the reason box can't
have the "OK" after it, so by having it on bottom, it looks strange as all the
submit buttons are above it, making it look more optional and trivial.

This is a lot of patch revisions, I hope this the last one :) ...

attachment checkuser2.patch ignored as obsolete

Oh, and "get edits", no says "get edits (default)". The enter hacks didn't
require a separate reason box fortunetely.

One last thing to mention, dates show like "07:29, 19 December 2006" only. It no
longer formats it based on the user doing it, which resulted in numerous
different formats (which apparently somewhat annoying).

Improve hack for "enter" key

More predictable behavoir when not using enter

attachment checkuser2.patch ignored as obsolete

Improve hack for "enter" key

Diff against svn hopefully

Attached: