Page MenuHomePhabricator

[EPIC] Core should provide inline diffs as well as side by side (Move InlineDifferenceEngine into core / remove MobileDiff)
Closed, ResolvedPublic

Assigned To
None
Authored By
Jdlrobson
Oct 30 2015, 7:45 PM
Referenced Files
F38450324: Screenshot_20231016_143926_Firefox.jpg
Oct 16 2023, 6:48 PM
F37161102: Screenshot 2023-08-02 at 5.23.17 PM.png
Aug 3 2023, 1:34 AM
F31518826: Screenshot_20200121-090309.png
Jan 21 2020, 3:43 AM
F31518822: Screenshot_20200121-090348.png
Jan 21 2020, 3:43 AM
F29716062: image.png
Jul 9 2019, 12:03 AM
Tokens
"Burninate" token, awarded by Nux."Like" token, awarded by Demian."The World Burns" token, awarded by xSavitar."The World Burns" token, awarded by Liuxinyu970226."The World Burns" token, awarded by jm3.

Description

Background

MobileFrontend provides an InlineDifferenceEngine which rather than displaying the diff in a 2 column layout displays it in a single column (e.g. unified). This seems like it would be useful for core as most websites these days offer unified as well as 2 column layout diffs and the former is extremely useful on mobile.

The Mobilefrontend diff has been a nuisance point in recent changes to revisions (e.g. Multi content revisions) and it would be helpful to make unified diffs a first class citizen in some form. This inconsistency causes confusion due to the completely different implementations: (see T123413)

Note there is an increasing amount of UI problems with this page. For example, the bytes added icon is misaligned with text (T340262#9065036)

Screenshot 2023-08-02 at 5.23.17 PM.png (1×988 px, 103 KB)

Proposal

Next steps

Questions

  • What other solutions provide inline diffs? Is MobileFrontend implementation the best one?
  • What refactors are necessary in core / to mobile code to support such a change if the mobile implementation is favoured.

Related Objects

StatusSubtypeAssignedTask
Declineddchen
OpenNone
OpenNone
DuplicateNone
OpenFeatureNone
OpenBUG REPORTNone
OpenNone
StalledNone
OpenFeatureNone
DuplicateNone
ResolvedNone
OpenNone
OpenNone
OpenFeatureNone
OpenNone
ResolvedNone
ResolvedNone
OpenFeatureNone
OpenNone
ResolvedHalfak
OpenNone
OpenNone
OpenFeatureNone
StalledNone
OpenNone
OpenNone
OpenNone
ResolvedPetrb
OpenNone
OpenNone
Resolvedtstarling
OpenNone
DeclinedNone
ResolvedBUG REPORTJdlrobson
ResolvedNone
Resolvedovasileva
ResolvedJdlrobson
ResolvedAmmarpad
OpenNone
ResolvedJdlrobson
ResolvedJdlrobson
Resolvedphuedx
ResolvedJdlrobson
Resolvedovasileva
Resolved marcella
Resolvedmatmarex
ResolvedBUG REPORTEsanders
Resolvedovasileva
Resolvedovasileva
ResolvedJdlrobson
InvalidNone
Resolved jkroll
OpenFeatureNone
OpenNone
ResolvedJdlrobson
ResolvedJdlrobson
ResolvedJdlrobson
DeclinedNone
ResolvedNone
ResolvedBUG REPORTSBisson
ResolvedJdlrobson
ResolvedBUG REPORTJdlrobson
ResolvedJSengupta-WMF
OpenBUG REPORTNone
OpenBUG REPORTNone
ResolvedJdlrobson

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 547766 abandoned by Jdlrobson:
[mediawiki/skins/MinervaNeue@master] Style the desktop diff page of Minerva to look like Special:MobileDiff

Reason:
Not much bandwidth for this right now, but happy to provide code review for efforts pushing us in that direction.

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

LGoto subscribed.

Sorry moved this on accident, please disregard.

Change 962091 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Distinguish between table and inline diffs

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

Change 962095 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/MinervaNeue@master] Style the desktop diff page of Minerva to look like Special:MobileDiff

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

Change 962091 merged by jenkins-bot:

[mediawiki/core@master] DifferenceEngine: Distinguish between table and inline diffs

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

Change 963393 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] DifferenceEngine: Comments on diff page should be themeable

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

Change 963395 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] DifferenceEngine: Make timestamps data readable

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

Change 963395 merged by jenkins-bot:

[mediawiki/core@master] DifferenceEngine: Make timestamps data readable

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

Change 963393 merged by jenkins-bot:

[mediawiki/core@master] DifferenceEngine: Comments on diff page should be themeable

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

Consider hiding or surfacing side by side diffs in mobile

I would like to have side by side available as an option on mobile, since inline diffs are sometimes an illegible mess of alternating words, e.g. an extreme case in https://en.m.wikipedia.org/wiki/Special:MobileDiff/1180439256:

Screenshot_20231016_143926_Firefox.jpg (2×1 px, 695 KB)

The process I used to use to switch from mobile diff to desktop diff used to be quite tedious, but I realized just a moment ago that clicking on "Desktop" works perfectly and takes just one click! So that's good enough for me.

I would like to have side by side available as an option on mobile, since inline diffs are sometimes an illegible mess of alternating words, e.g. an extreme case in https://en.m.wikipedia.org/wiki/Special:MobileDiff/1180439256: F38450324

The way this inline diff is presented, is really unhelpful. The algorithm may try to choose common words in both sides to group changes together. But the way it's choosing "spaces" to group them, splitting word by word instead of presenting the entire edit as a single deletion + insertion group, is very unreasonable. It seems like wikEdDiff works better for this, as the example provided in the description of T346460.

Sometimes the spaces can be the LCS, so they are highlighted as the context and all the words are shown as differences. See T326773: Wikidiff2 considers each space to be a separate word for my thoughts on this.

Jdlrobson updated the task description. (Show Details)