Page MenuHomePhabricator

Include section name in the revision history visual diff
Closed, ResolvedPublic

Description

As in the parent task (T231698), but just for the visual diff.

Proposed screenshot from project page:

testing

Check diffs where the change is at least one paragraph after the section heading, such as

Event Timeline

Change 644073 had a related patch set uploaded (by Esanders; owner: Esanders):
[VisualEditor/VisualEditor@master] Always show heading context in visual diff

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

Change 644073 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Always show heading context in visual diff

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

matmarex added a subscriber: matmarex.

We haven't really tested or reviewed this, or at least I didn't… Ed wrote some unit tests thought, so it probably works.

Change 645472 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (fc368ac88)

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

Change 645472 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (fc368ac88)

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

Ryasmeen renamed this task from Include section name in the visual diff to Include section name in the revision history visual diff.Dec 14 2020, 11:07 PM

The parent task was 9th in https://meta.wikimedia.org/wiki/Community_Wishlist_Survey_2019/Results so commtech may want to announce it has been partially resolved.

Thanks, @Esanders! I have updated the 2019 results page and the project page for the wish with this update.

Hello, @Esanders. I'm keeping eye on this as original proposer. So I've tryed, and I can't see any difference:



If it was merged to 1.36.0-wmf.21, shouldn't it already work?

Looks like there are still some issues with inline changes.

Change 649899 had a related patch set uploaded (by Esanders; owner: Esanders):
[VisualEditor/VisualEditor@master] Follow-up I9f136620f: Fix typo in section diff code

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

Change 649899 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Follow-up I9f136620f: Fix typo in section diff code

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

Change 650013 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (dd92d178d)

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

Change 650013 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (dd92d178d)

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

Checked on production (en.wiki) on some articles with the visual diff containing at least one paragraph of change.

ppelberg claimed this task.

Hello, @Esanders. It looks different now, but has a lot of problems. It shows dozens of irrelevant paragraphs, and also shows empty section names.

@IKhitron Can you link to a few examples? I just looked at about 20 diffs from my watchlist and didn't see such problems. (I filed T272314 and T272315 though…)

@IKhitron Can you link to a few examples? I just looked at about 20 diffs from my watchlist and didn't see such problems. (I filed T272314 and T272315 though…)

Sure, I'll try to find some. It can take time, because I use usually the wikicode diff.
Here is the first one: link. It has many problems, let's pick one: the line
הועבר לדף שיחה:חיסון#חיסון והתחסנות
marked in the section
אפשרות ב'
when it should be in the section
חיסון והתחסנות
much lower.

Thanks, I can see that.

Screenshots for reference:

I think this is expected. It appears this way because this line is shown as part of the context for the second part of the diff (it wasn't changed in these edits), and the additional section names are only shown for changed text. The name of the previous section also appears because it's part of the context for the first part of the diff.

It actually appears in the same way in the wikitext diff:


In this picture, I drew three boxes around the three parts of the diff. Each part contains the actual changes, and some context before and after. Only the one section name visible between the boxes is added as a result of this task; the other ones are included in the "normal" diff context.

I think this diff looks weird because the "normal" context is so large (because it's a discussion page, which uses HTML lists for comments, and the entire list is included).

This is a bit ugly, but I think the diff is correct (and it's definitely not a bug caused by the changes to handling section names, it might be a separate issue).

Maybe we should display the "extra" section names differently, to distinguish them from the "normal" context?

I do not say if the first name should or should not be here. I do say the second name should be here for sure, the last header before the quoted line, and it's not here at all.

Right. So, in other words, I think you're saying that a section name should be displayed not only if the changed text belongs to that section, but also if any of the "normal" context text belongs to that section. I'm not sure what is better.

No. What I'm saying that if you display a part of section from any reason, you should not give her a wrong name. Nkw one can understand the line is the part of the wrong section. You can show both names, it will solve the problem. Or you can find another visualization.

Another possible solution: The line above was added for better orientation. Now, when we have section names, the line above the header is not needed any more.