Page MenuHomePhabricator
Create Task
Maniphest T192755

Simply cloning diff header disrupts user scripts and causes other issues
Closed, ResolvedPublic
Actions

Description

For T187614: Metadata (author, edit comment, navigation links etc) not shown in historic visual diff, the Visual Diff clones the header of the diff. While this works, there are some problems with this approach:

  1. It will clone any tr.diff-title, even if it is some user-generated content from the article shown below.
  2. This results in duplicated IDs, e.g. #mw-diff-otitle1, which is both invalid and could cause problems with any of these and similar scripts.

These issues aren't just hypothetic ones, I struggled with both while updating my script https://de.wikipedia.org/wiki/Benutzer:Schnark/js/diff.js, which adds a third diff mode. (Though due to T171437 this script already has a lot of hacks, so it didn't really matter to add even more.)

Other bug reports:

Details

Related Changes in Gerrit:
SubjectAuthorRepoBranchLines +/-
ve.init.mw.DiffPage.init: Do not duplicate diff table headerJforrestermediawiki/extensions/VisualEditorwmf/1.32.0-wmf.1+16 -15
ve.init.mw.DiffPage.init: Do not duplicate diff table headerBartosz Dziewońskimediawiki/extensions/VisualEditormaster+16 -15
Customize query in gerrit

Related Objects

Event Timeline

Schnark created this task.Apr 23 2018, 7:07 AM
Restricted Application added a project: VisualEditor. · View Herald TranscriptApr 23 2018, 7:07 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
matmarex claimed this task.EditedApr 24 2018, 5:43 PM
matmarex subscribed.

I suppose we could move it instead of duplicating it, would that work?

• Deskana triaged this task as Medium priority.Apr 24 2018, 6:55 PM
• Deskana edited projects, added: VisualEditor (Current work); removed: VisualEditor.
• Deskana moved this task from Incoming to In progress on the VisualEditor (Current work) board.
Schnark added a comment.Apr 25 2018, 9:32 AM

Alternatively one could wrap the header in <thead>, and the actual diff in <tbody> and then only hide the body when switching to visual (and toggle the data-attributes).

TheDJ merged a task: T193218: The "Visual differences" beta feature outputs two different HTML div with same ID.Apr 27 2018, 11:47 PM
TheDJ added subscribers: EtaoinWu, TheDJ, Xiplus.

Many people complaining about missing twinkle links in the diff view. https://en.wikipedia.org/wiki/Wikipedia_talk:Twinkle#Missing_Rollback_Options_in_Diff_View

Jc86035 subscribed.Apr 28 2018, 11:02 AM
Izno subscribed.Apr 28 2018, 8:27 PM
MusikAnimal subscribed.Apr 29 2018, 1:30 AM
• Deskana subscribed.Apr 30 2018, 11:20 AM
In T192755#4165632, @TheDJ wrote:

Many people complaining about missing twinkle links in the diff view. https://en.wikipedia.org/wiki/Wikipedia_talk:Twinkle#Missing_Rollback_Options_in_Diff_View

Thanks. This is in progress, so hopefully a fix should be here soon.

• Deskana renamed this task from Simply cloning diff header doesn't seem like a good idea to Simply cloning diff header disrupts user scripts and causes other issues.Apr 30 2018, 1:25 PM
• Deskana merged a task: T193397: Visual differences beta feature breaks Twinkle.
• Deskana added a subscriber: Waddie96.
Liuxinyu970226 subscribed.Apr 30 2018, 2:58 PM
gerritbot subscribed.Apr 30 2018, 5:24 PM

Change 429845 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] ve.init.mw.DiffPage.init: Do not duplicate diff table header

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

gerritbot added a project: Patch-For-Review.Apr 30 2018, 5:24 PM
matmarex added a comment.Apr 30 2018, 5:25 PM

Sorry, I didn't anticipate this breaking other scripts, and I wasn't checking my bugmail over the weekend. I will try to get the fix deployed later today.

Jdforrester-WMF moved this task from In progress to Code review on the VisualEditor (Current work) board.Apr 30 2018, 5:33 PM
Jdforrester-WMF added a project: VisualEditor-MediaWiki.Apr 30 2018, 9:23 PM
gerritbot added a comment.Apr 30 2018, 9:26 PM

Change 429845 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] ve.init.mw.DiffPage.init: Do not duplicate diff table header

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

gerritbot added a comment.Apr 30 2018, 9:28 PM

Change 429986 had a related patch set uploaded (by Jforrester; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@wmf/1.32.0-wmf.1] ve.init.mw.DiffPage.init: Do not duplicate diff table header

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

Jdforrester-WMF subscribed.Apr 30 2018, 9:29 PM

I've scheduled this for SWAT in 90 minutes' time.

matmarex updated the task description. (Show Details)Apr 30 2018, 9:35 PM
ReleaseTaggerBot added a project: MW-1.32-notes (WMF-deploy-2018-05-01 (1.32.0-wmf.2)).Apr 30 2018, 10:00 PM
matmarex raised the priority of this task from Medium to High.Apr 30 2018, 10:24 PM
gerritbot added a comment.Apr 30 2018, 11:24 PM

Change 429986 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@wmf/1.32.0-wmf.1] ve.init.mw.DiffPage.init: Do not duplicate diff table header

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

Stashbot subscribed.Apr 30 2018, 11:38 PM

Mentioned in SAL (#wikimedia-operations) [2018-04-30T23:38:01Z] <catrope@tin> Synchronized php-1.32.0-wmf.1/extensions/VisualEditor/modules/ve-mw/init/ve.init.mw.DiffPage.init.js: T192755 (duration: 00m 59s)

Jdforrester-WMF moved this task from Code review to Product owner review on the VisualEditor (Current work) board.Apr 30 2018, 11:43 PM
Jdforrester-WMF removed a project: Patch-For-Review.

Now deployed everywhere.

ReleaseTaggerBot edited projects, added: MW-1.32-notes (WMF-deploy-2018-04-24 (1.32.0-wmf.1)); removed: MW-1.32-notes (WMF-deploy-2018-05-01 (1.32.0-wmf.2)).May 1 2018, 12:01 AM
• Deskana added a comment.May 1 2018, 8:10 AM
In T192755#4168754, @matmarex wrote:

Sorry, I didn't anticipate this breaking other scripts, and I wasn't checking my bugmail over the weekend. I will try to get the fix deployed later today.

It's fine. It's a beta feature, so things like this will break sometimes.

• Deskana closed this task as Resolved.May 1 2018, 8:12 AM
Liuxinyu970226 unsubscribed.May 2 2018, 7:56 AM
Mainframe98 merged a task: T193218: The "Visual differences" beta feature outputs two different HTML div with same ID.Jul 1 2018, 9:01 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits