Page MenuHomePhabricator

Inconsistent separators in Watchlist link toolbars with "enhanced recent changes" (patch)
Closed, ResolvedPublic

Description

Author: happy.melon.wiki

Description:
With the recent changes to the way separators have been implemented (bug7509, r47046 and related) there is now an unresolvable inconsistency in the way they are used in watchlist entries. Without ERC, entries show up like this:

(diff) (hist) . . Glenn Quagmire‎; 17:29 . . (+18) . . 86.20.232.82 (talk | block) [rollback]

and With ERC, like this:

17:29 Glenn Quagmire‎ (diff; hist) . . (+18) . . 86.20.232.82 (talk | block) [rollback]

Note the *horrible* "diff; hist" construct that is now created. *BUT* that semicolon is from the same system message ([[MediaWiki:semicolon-separator]]) that produces the semicolon after the article title without ERC. The bug is that, with ERC, the diff/hist links should use the [[MediaWiki:pipe-separator]] system message for the separator. I've looked through the various code revs associated with these changes, and I can't get my head around where to find the relevant lines; assigning to siebrand 'cos he did them in the first place :D


Version: 1.15.x
Severity: enhancement
URL: http://en.wikipedia.org/w/index.php?title=Wikipedia:Village_pump_(technical)&oldid=279676639#Pipe_character_between_page_name_and_timestamp_in_watchlist

Details

Reference
bz18161

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:30 PM
bzimport set Reference to bz18161.
bzimport added a subscriber: Unknown Object (MLST).

Not accepting assignment. Reason: Enhanced RC is done by JS as far as I know, and I have never made changes in there.

I propose this solution:

(diff | hist) . . Glenn Quagmire‎; 17:29 . . (+18) . . 86.20.232.82 (talk | block)

and with ERC:

17:29 Glenn Quagmire‎ (diff | hist) . . (+18) . . 86.20.232.82 (talk | block)

happy.melon.wiki wrote:

@siebrand: I know some of it is done with JS, but there is also a significant amount of different stuff that comes out of the PHP base. JS is AFAIK just for collapsing the multiple operations together.

@Raimond: I generally agree, but A) that then makes the display without ERC different to the appearance on page histories (unless you want to change them too), and B) is a separate issue to fixing the diff/hist links with ERC.

Did a quick check on 1.13 and 1.14 installations...

Standard RC looks like:
(diff) (hist) . . Main Page‎; 16:44 . . (+9) . . WikiSysop (Talk | contribs | block)

Enhanced RC looks like:
Main Page‎ (diff; hist) . . (+9) . . WikiSysop (Talk | contribs | block)

This is exactly what it looks like now, so apparently there has not in fact been a change; the basis of this bug is incorrect. Resolving INVALID.

happy.melon.wiki wrote:

Ok, so it's not a recent change, that explains why I couldn't find it in any of Siebrand's commits. It's still Wierd(TM). So remove the dependency, make it an enhancement, and say that it should be *internally* consistent with the "talk/contribs/block" and "rollback/undo" toolbars. Or it should be two separate brackets like without ERC. But it should definitely not be AFAIK the only place on-wiki (apart from RC which uses the same code) where a bracketed toolbar is separated by a semicolon.

happy.melon.wiki wrote:

one-line change to ChangesList.php

Found it; one-line change at the end of the longest call stack I've seen for a while (no wonder the HTML output of the watchlist is such a mess!).

attachment bug18161.txt ignored as private

happy.melon.wiki wrote:

updated patch, against r53416

Original patch is 11,000 revisions out of date now :S Updated patch and tested against r53416.

Attached: