Page MenuHomePhabricator

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

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
File Not Attached
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

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
OpenBUG REPORTNone
OpenNone
Resolvedovasileva
StalledNone
ResolvedAmmarpad
OpenNone
StalledNone
ResolvedJdlrobson
Resolvedphuedx
ResolvedJdlrobson
Resolvedovasileva
Resolved marcella
Resolvedmatmarex
ResolvedBUG REPORTEsanders
Resolvedovasileva
Resolvedovasileva
ResolvedJdlrobson
OpenNone
Resolved jkroll
StalledFeatureNone
OpenNone
StalledNone
ResolvedJdlrobson
ResolvedJdlrobson
OpenNone
OpenNone

Event Timeline

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

Does this still need code review from Platform Engineering?

Not right now but input (guidance) is welcomed from TechCom (and core platform) on T240608

I am seeing some regressions in this change. I have created a subtask.

Another regression:

Structured data edit diffs are still being served side by side rather than inline if diff-type=inline is set

Steps to reproduce

Actual results

  • Structured data edit diffs are still being served side by side even if diff-type=inline is set.

Expected results

  • We see a unified diff.

View https://commons.wikimedia.org

Note we don't support the new diff type officially on desktop. That's blocked on T240608.

I think this is an issue with mobile however?
https://commons.m.wikimedia.org/wiki/Special:MobileDiff/388741403

Thanks for the bug report I've opened T243235 to explore.

As I explained in,
T117279#5817692, I am seeing something like this on desktop mode. The diff is not shown unified even if I set diff-type=inline in the URL. The below screenshot is of https://commons.wikimedia.beta.wmflabs.org/w/index.php?title=File:Photo_on_14-01-2020_at_11.33.jpg&diff=216482&diff-type=inline&diffmode=source.

Screenshot_20200121-090348.png (480×960 px, 97 KB)

Similar problem on mobile. On mobile a structured data diff should be unified to be consistent with the normal wikitext diff. But I see side by side diffs. The below screenshot is of https://commons.m.wikimedia.beta.wmflabs.org/wiki/Special:MobileDiff/216482.
Screenshot_20200121-090309.png (480×960 px, 45 KB)

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 added a subscriber: LGoto.

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.

Nux added a subscriber: Nux.