We are also showing the switcher when doing a diff while editing ("Show Changes"). If you try to switch to the "Visual" diff (or have it already selected) you get an "Invalid response from server" error.

This does not affect Live Preview.

Change 914429 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@wmf/1.41.0-wmf.7] Remove duplicated diff-mode selector in save dialog

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

Mentioned in SAL (#wikimedia-operations) [2023-05-03T07:18:40Z] <taavi@deploy1002> Started scap: Backport for [[gerrit:914429|Remove duplicated diff-mode selector in save dialog (T324759)]]

Mentioned in SAL (#wikimedia-operations) [2023-05-03T07:20:13Z] <taavi@deploy1002> taavi and samwilson: Backport for [[gerrit:914429|Remove duplicated diff-mode selector in save dialog (T324759)]] synced to the testservers: mwdebug2001.codfw.wmnet, mwdebug1001.eqiad.wmnet, mwdebug1002.eqiad.wmnet, mwdebug2002.codfw.wmnet

Mentioned in SAL (#wikimedia-operations) [2023-05-03T07:28:55Z] <taavi@deploy1002> Finished scap: Backport for [[gerrit:914429|Remove duplicated diff-mode selector in save dialog (T324759)]] (duration: 10m 14s)

Change 915365 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/extensions/VisualEditor@master] Don't show the diff-mode selector if not viewing a diff

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

We are also showing the switcher when doing a diff while editing ("Show Changes"). If you try to switch to the "Visual" diff (or have it already selected) you get an "Invalid response from server" error.

The above patch prevents the switcher from being shown for 'Show Changes', which avoids the issue. As far as I know, we're not planning on enabling switching visual/source while viewing changes in the wikitext editor. The switcher should only be shown when viewing a diff or in VE's save dialog when viewing changes.

Change 915365 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Don't show the diff-mode selector if not viewing a diff

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

Further to T324759#8722735, I tested the alignment of the legend and switcher in 5 skins with various zoom levels on:

• Safari on enwiki beta
• Firefox on arwiki beta (to test RTL behaviour)

I didn't notice any alignment issues. The switcher and legend are now vertically aligned.

I checked that they interacted fine with RevisionSlider and FlaggedRevs on dewiki beta (e.g. https://de.wikipedia.beta.wmflabs.org/w/index.php?diff=25353&oldid=25352&title=Energie&diff-type=inline).

In T324759#8842876, @gerritbot wrote:

Change 915365 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Don't show the diff-mode selector if not viewing a diff

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

This appears to fix the bug described in T324759#8822493. It also fixes the bug when doing a rollback or undo action.

I checked as many different types of diffs as I could find to see that the legend or switcher do not appear when they shouldn't. I tested everything here except mobile app, SpamDiffTool and WikEdDiff.

We still seem to have a bug with Translatewiki. See T336262#8843883. I will continue testing there and move this task along.

I also raised T336480, T336481 and T336482 for less urgent bugs.

Locally, I also tested:

• wiki with VisualEditor but not wikidiff2
• wiki with neither VE nor wikidiff2

Change 921773 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/core@master] diff: Add legend and tooltips to inline diff display

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

The following tickets will be closed since this new approach will solve the problems described in this tasks:

• T336482: Legend appears on Special:MobileDiff when viewed on desktop
• T336582: Disable visual diffs in Special:AbuseFilter diffs
• T336262: "Visual" and "Wikitext" buttons in diffs in the Translate UI

T336909: Logic for VE diff switch should have an inclusive check is no longer valid since a different solution in patch is being implemented.

In T324759#8874993, @HMonroy wrote:

The following tickets will be closed since this new approach will solve the problems described in this tasks:

• T336482: Legend appears on Special:MobileDiff when viewed on desktop
• T336582: Disable visual diffs in Special:AbuseFilter diffs

T336909: Logic for VE diff switch should have an inclusive check is no longer valid since a different solution in patch is being implemented.

@Samwilson please let me know if this makes sense :) Thank you!

Yes, I think this is right.

I do wonder if we should reorganise the patches a bit, e.g.:

1. add the new DifferenceEngineBeforeDiffTable hook, but with nothing in it (so it'd not change anything);
2. update VE to use the new hook, to move the existing mode switch;
3. add the type toggle to core, passing it in the hook's $parts paramater;
4. add the legend, also in $parts.

That way, we don't have the awkward situation of things being implemented before they can be used (the legend) or of needing to merge extension and core patches at the same time in order for them to work properly and not break the display.

In T324759#8875363, @Samwilson wrote:

Yes, I think this is right.

I do wonder if we should reorganise the patches a bit, e.g.:

1. add the new DifferenceEngineBeforeDiffTable hook, but with nothing in it (so it'd not change anything);
2. update VE to use the new hook, to move the existing mode switch;
3. add the type toggle to core, passing it in the hook's $parts paramater;
4. add the legend, also in $parts.

That way, we don't have the awkward situation of things being implemented before they can be used (the legend) or of needing to merge extension and core patches at the same time in order for them to work properly and not break the display.

As discussed during our collaboration meeting, we'll continue with this approach and revert the MobileFrontend and VE patches.

Change 922640 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/extensions/VisualEditor@master] Move the diff-mode selector to the new DifferenceEngineBeforeDiffTable hook

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

Change 922789 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/extensions/MobileFrontend@master] diff: Hide legend for inline diffs

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

Test wiki created on Patch demo by HMonroy (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/260273a763/w

The three patches for this can be QA'd on patchdemo at e.g. https://patchdemo.wmflabs.org/wikis/bb99dc6987/w/index.php?title=Main_Page&diff=151&oldid=1&diff-type=inline

The patches are ready for review, but shouldn't be +2'd until QA is done.

@Samwilson Was it our intention to add the tooltips to the Special:MobileDiff wikitext view? e.g. https://patchdemo.wmflabs.org/wikis/bb99dc6987/wiki/Special:MobileDiff/164

In T324759#8891469, @dom_walden wrote:

@Samwilson Was it our intention to add the tooltips to the Special:MobileDiff wikitext view? e.g. https://patchdemo.wmflabs.org/wikis/bb99dc6987/wiki/Special:MobileDiff/164

Yes, or rather it's intentional not to remove them. :-) They don't serve much purpose on mobile, but don't do any harm either.

I have tested on patchdemo as many of the diffs from the list as I can (although the ~15 items from "Wikibase/WikibaseLexeme" to "EntitySchema" cannot be tested on patchdemo). I have also tested diffs of revisions from a small number of different content types (e.g. javascript, json, wikitext).

As far as I can remember, I only saw the switcher in the case of desktop or mobile diff (accessed from revision history, not during editing) or Special:ComparePages, and only if the revision on the right (in two column view) is of type wikitext. (Interestingly, you can diff revisions of different pages and content types).

I noticed a few bugs but I don't think they are related to the current work. I will raise them separately.

To be able to test more of the diffs from the list and other things like multi-content revisions, it would be useful to have this on beta cluster. Therefore, I am happy for this to be merged.

Change 921773 merged by jenkins-bot:

[mediawiki/core@master] diff: Add legend and tooltips to inline diff display

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

Change 922640 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Move the diff-mode selector to the new TextSlotDiffRendererTablePrefix hook

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

Change 922789 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] diff: Hide legend for inline diffs

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

@HMonroy @dmaza I tested almost all the diffs from the list on beta. I don't think the switcher and legend are appearing in any place they should not (with the exception noted below).

On Commons beta, I was able to test Multi-Content Revisions. For example, diffs which show the mediainfo slot (https://commons.wikimedia.beta.wmflabs.org/w/index.php?title=File%3A123_4.jpg&diff=289284&oldid=280359) or both main and mediainfo (https://commons.wikimedia.beta.wmflabs.org/w/index.php?diff=289821&oldid=178021). I tested MCR revisions for as many different types of diffs as I could.

In the case where the diff only shows a mediainfo slot (e.g. https://commons.wikimedia.beta.wmflabs.org/w/index.php?title=File%3A123_4.jpg&diff=289284&oldid=280359&diff-type=inline) the legend does appear in inline mode even though we don't support inline mode for that particular slot/role, instead displaying it in two column mode (see below). Perhaps the legend can also work for the two column diff, although the shade of blue and yellow is not an exact match. Of course, the two column mode does not have the tooltips.

The switcher also appears even though Visual diffs does not seem to support mediainfo (no error is returned but no diff is displayed either), although this is pre-existing behaviour.

In the case of a diff which has both the main and mediainfo slots (e.g. https://commons.wikimedia.beta.wmflabs.org/w/index.php?diff=289821&oldid=178021&diff-type=inline) we also show the switcher and legend. This makes more sense although we only show inline diff for the main slot.

It should also be noted that the tooltips are added to crossed-out and underlined changes, as shown below (example). This happens when a paragraph is moved and words added/removed from it. @JSengupta-WMF is this OK?

I was not able to test the Translate extension, SpamDiffTool or WikEdDiff.

@GMikesell-WMF might have more to add.

Test environment: https://en.wikipedia.beta.wmflabs.org and https://commons.wikimedia.beta.wmflabs.org

• MediaWiki 1.41.0-alpha (971e2cf) 09:40, 7 June 2023.
• VisualEditor 0.1.2 (ed3f643) 23:55, 6 June 2023.
• MobileFrontend 2.4.1 (10a974e) 18:27, 6 June 2023.

Test browser: Firefox 102.

@HMonroy @dmaza Nothing else to add from Dom's except a couple of things below.

OS: macOS 13.3
Chrome: 113

• Diff of suppressed revisions - I was getting a invalid response from server error on diff of suppresses revisions as seen in the screenshot below.
  • https://en.wikipedia.beta.wmflabs.org/w/index.php?title=Conflict-title-0.25968288104487747-I%C3%B1t%C3%ABrn%C3%A2ti%C3%B4n%C3%A0liz%C3%A6ti%C3%B8n&diff=483921&oldid=483920&unhide=1
  • https://en.wikipedia.beta.wmflabs.org/w/index.php?title=Conflict-title-0.25968288104487747-I%C3%B1t%C3%ABrn%C3%A2ti%C3%B4n%C3%A0liz%C3%A6ti%C3%B8n&diff=483921&oldid=483920&unhide=1&diff-type=inline

FlaggedRevs

• https://de.wikipedia.beta.wmflabs.org/w/index.php?title=MBq-Testseite-1&action=edit

Show Changes - shows no difference when there is a change?

Show Preview- no marks on the changes?

@dom_walden first two cases are ok. Color on inline and two column diffs are different and the legends are matched with the inline diff colors. This is a larger issue that needs to be addressed with the design systems team and it's in their backlog. For the third case, I'm not sure why word addition/deletion on a moved paragraph is denoted by underline/strikethrough. Is it a known problem? They should also be shown with blue/yellow highlights.

In T324759#8917958, @JSengupta-WMF wrote:

...For the third case, I'm not sure why word addition/deletion on a moved paragraph is denoted by underline/strikethrough. Is it a known problem? They should also be shown with blue/yellow highlights.

I believe it is part of the existing design. Not sure if it is specified anywhere though. I don't know the technical implications of changing it.

Change 930279 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] diff: Move SlotDiffRenderer::getTablePrefix() parts assembly up to DifferenceEngine

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

In T324759#8933428, @gerritbot wrote:

Change 930279 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] diff: Move SlotDiffRenderer::getTablePrefix() parts assembly up to DifferenceEngine

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

I have nothing else to add to my testing from T324759#8909457. Moving back into code review.

Change 932351 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/core@master] diff: Add inline styles for changes within moved paragraphs

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

For the third case, I'm not sure why word addition/deletion on a moved paragraph is denoted by underline/strikethrough. Is it a known problem? They should also be shown with blue/yellow highlights.

I've made a patch to fix this, it's ready for review.

Change 930279 merged by jenkins-bot:

[mediawiki/core@master] diff: Move SlotDiffRenderer::getTablePrefix() parts assembly up to DifferenceEngine

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

Change 932351 merged by jenkins-bot:

[mediawiki/core@master] diff: Add inline styles for changes within moved paragraphs

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

@Samwilson On beta, the mobile visual diff is showing both the wikitext and visual diff. For example, https://en.m.wikipedia.beta.wmflabs.org/w/index.php?title=Special:MobileDiff/453540&diffmode=visual

Just a note for @JSengupta-WMF. In the mobile wikitext diff, inserts/deletes inside moves are now blue/yellow but in other places are green/red. For example, https://en.m.wikipedia.beta.wmflabs.org/wiki/Special:MobileDiff/586781

Previously, they used underline/strikeout, like with inline diffs previously (example https://en.m.wikipedia.org/wiki/Special:MobileDiff/1161994862).

Change 933071 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/core@master] Switch back to oo-ui-element-hidden from mw-diff-element-hidden

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

Change 933074 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/extensions/VisualEditor@master] Switch back to oo-ui-element-hidden from mw-diff-element-hidden

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

Change 933071 merged by jenkins-bot:

[mediawiki/core@master] Switch back to oo-ui-element-hidden from mw-diff-element-hidden

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

Change 933074 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Switch back to oo-ui-element-hidden from mw-diff-element-hidden

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

On beta, the mobile visual diff is showing both the wikitext and visual diff. For example, https://en.m.wikipedia.beta.wmflabs.org/w/index.php?title=Special:MobileDiff/453540&diffmode=visual

Thanks for pointing this out @dom_walden, the above patches fix this and it should be working as expected now.

Just a note for @JSengupta-WMF. In the mobile wikitext diff, inserts/deletes inside moves are now blue/yellow but in other places are green/red. For example, https://en.m.wikipedia.beta.wmflabs.org/wiki/Special:MobileDiff/586781
Previously, they used underline/strikeout, like with inline diffs previously (example https://en.m.wikipedia.org/wiki/Special:MobileDiff/1161994862).

This is because of change 932351 that I made above. I've made a new one to made the similar fix to MobileFrontend.

Change 933212 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/extensions/MobileFrontend@master] diff: Add inline styles for changes within moved paragraphs

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

In T324759#8963433, @dom_walden wrote:

Just a note for @JSengupta-WMF. In the mobile wikitext diff, inserts/deletes inside moves are now blue/yellow but in other places are green/red. For example, https://en.m.wikipedia.beta.wmflabs.org/wiki/Special:MobileDiff/586781

Previously, they used underline/strikeout, like with inline diffs previously (example https://en.m.wikipedia.org/wiki/Special:MobileDiff/1161994862).

@dom_walden for mobile we decided to keep the design as it is (Red and green). Did the web implementation break it? It should be red/freen on mobile and yellow/blue on desktop for now.

Change 933212 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] diff: Add inline styles for changes within moved paragraphs

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

Moves on the mobile diff are now red/green.

I retested all the different types of diffs. I didn't notice any problems.

Examples:

• https://en.m.wikipedia.beta.wmflabs.org/wiki/Special:MobileDiff/586781
• https://meta.wikimedia.beta.wmflabs.org/w/index.php?title=User%3ADom_walden%2Ffoo.js&diff=48640&oldid=48638&diff-type=inline
• https://commons.wikimedia.beta.wmflabs.org/w/index.php?title=File%3AIcon_for_char_%E8%A6%8F_purple_beige_32x32.png&diff=290346&oldid=114026&diff-type=inline

Test wiki on Patch demo by GMikesell-WMF using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/d08cff28d9/w/

Test wiki on Patch demo by GMikesell-WMF using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/afa351f7c9/w/

Test wiki on Patch demo by HMonroy (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/260273a763/w/