Page MenuHomePhabricator

Revision Slider compares the same revision
Closed, ResolvedPublic

Description

When moving the blue pointer to the same bar in which the yellow pointer is already located, the "differences" between the same revision appear.

Here is a short video demonstrating it:
https://www.youtube.com/watch?v=SUXYE1TyWLY

Is this intentional? I think it's useless to "compare" the same revision.

Event Timeline

Guycn2 created this task.Jul 25 2016, 11:27 AM
Restricted Application added a project: TCB-Team. · View Herald TranscriptJul 25 2016, 11:27 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

@Guycn2, could you please use ?uselang=en, so we can understand what happens there (at least for screenshot, if video is too hard to make). Thank you.

@IKhitron, I don't think that understanding the display language is important for understanding the issue, but here is a screenshot with uselang=en :

BTW, here's another bug: The diff page partially ignores uselang=en. It's probably related to the revision slider.

@Guycn2 thanks for reporting this.

Currently it is expected behaviour. It indeed does not make much sense to compare a revision with itself.
But given the UI the extension uses (draggable pointers that could be moved and dropped on any revision) it seems (at lest to me) there is no reasonable way not to allow user to compare the revision to itself.
The history page lists revisions with a bit different UI that prevents from that but in RevisionSlider case that is not that (easily) possible.
But I think the current behaviour (showing a "diff" with no differences) is at least correct in that sense it actually shows there is no difference between revision N and N.

But as this has already been reported to us that users find this confusing (I would swear I've seen a Phabricator task about this already but cannot find it right now), we will consider how to change/improve this particular part of UI.

@Guycn2 thanks for reporting this.
Currently it is expected behaviour. It indeed does not make much sense to compare a revision with itself.
But given the UI the extension uses (draggable pointers that could be moved and dropped on any revision) it seems (at lest to me) there is no reasonable way not to allow user to compare the revision to itself.
The history page lists revisions with a bit different UI that prevents from that but in RevisionSlider case that is not that (easily) possible.
But I think the current behaviour (showing a "diff" with no differences) is at least correct in that sense it actually shows there is no difference between revision N and N.
But as this has already been reported to us that users find this confusing (I would swear I've seen a Phabricator task about this already but cannot find it right now), we will consider how to change/improve this particular part of UI.

Thank you for the answer.

It would be better if a popup or an error message would appear when a user tries to compare a revision with itself.

Addshore moved this task from Incoming to Revision Slider on the TCB-Team board.Jul 25 2016, 7:54 PM
Lea_WMDE moved this task from Incoming to Backlog on the Revision-Slider board.Aug 2 2016, 9:49 AM

Hi @Guycn2, I guess this issue comes down to finding out if people who put both pointers on the same revision know what that means or not. If they know, we would not have to help them out of the situation. If they get scared of an empty diff, we should think of something. Did you yourself get confused when you saw an empty diff? If yes, how did you end up there? Did you accidentally move both pointers to the same revision or did you test for that situation?

Guycn2 added a comment.EditedAug 3 2016, 7:22 PM

Hi @Guycn2, I guess this issue comes down to finding out if people who put both pointers on the same revision know what that means or not. If they know, we would not have to help them out of the situation. If they get scared of an empty diff, we should think of something. Did you yourself get confused when you saw an empty diff? If yes, how did you end up there? Did you accidentally move both pointers to the same revision or did you test for that situation?

@Lea_WMDE, It did not happen to me, but a user from hewiki complained about it.

I think it's kind of weird when an empty diff appears. It might be unclear to new RevisionSlider users, because there is no way to see an empty diff without the RevisionSlider (except for page protections and moves).

Currently, I can think of a non-cumbersome constraint that prevents that from happening in the first place. We discussed that a while back and all solutions causes illogical and/or cumbersome cases.

What about making it more clear that you »compare« the same version? If you accidentally moved the slider to the same revision it may be irritating the the diff is "gone".

now:


small adjustment:

It seems that this is a part of the UI which is from the "normal" interface which we might not to change; on the other hand, the empty diff notification has an own CSS class (mw-diff-empty) already, so it might not be too hard.

Just to make "technical" details clear:

  • This text saying "No differences" is indeed generated by core diff code. So technically speaking diff is not gone, there simply are not differences to show, and diff engine can perfectly deal with comparing identical revisions.
  • it is not impossible to get diff comparing the same revisions already without RevisionSlider. Simple ways to achieve this that directly come to my mind are: with JS disabled one can select the same revision on the history page, and revisions to compare can be always changed (ie. to be both the same revision) by manipulating the URL

But I agree both cases I mentioned are not very common cases!

  • it is not impossible to get diff comparing the same revisions already without RevisionSlider. Simple ways to achieve this that directly come to my mind are: with JS disabled one can select the same revision on the history page, and revisions to compare can be always changed (ie. to be both the same revision) by manipulating the URL

Indeed, so comparing 2 revisions that are actually the same revision is a case that we always need to cover. Personally I don't see an issue with how the RevisionSlider currently does this. I feel that doing anything else would be far more confusing.

Lea_WMDE closed this task as Declined.Aug 10 2016, 3:49 PM

As the RevisionSlider is not behaving illogically, I am closing this. Happy to open it again, when people who were disturbed by it (when stumbling over it) give more details as to what they intended to do, what they expected and what confused them.

Esanders reopened this task as Open.Aug 27 2016, 12:08 AM
Esanders added a subscriber: Esanders.

The UI definitely should prevent the user from creating this state. If the user isn't clear about on which side the diff boundaries are inclusive, they may try to align the sliders to get a one-revision diff. If the UI blocks it, it is clear that is not a valid selection.

The slider should simply remain in the last valid (i.e. non-conflicting) position. When the user's cursor eventually gets to the other side, the marker will jump two places. Basically how you would expect drag-and drop to handle an invalid drop location. You could also use CSS cursor not-allowed/no-drop.

The fact the slider can be initialised in this state also isn't much of a problem as you can just add an exception for that edge case (also the old history interface prevents this, so they'd probably have to hack the URL to end up in such a situation).

Change 315620 had a related patch set uploaded (by Esanders):
Prevent selecting the same revision twice

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

@Esanders Thanks!
The change works if you drag the arrows (and it feels "right" to me). The prevention does not take place if you click on the bars to move the slider. I am unsure about introducing this minor inconsistency, on the other hand, I currently can't imagine a huge problem with it.

Change 315620 merged by jenkins-bot:
Prevent selecting the same revision twice

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

Addshore closed this task as Resolved.Oct 20 2016, 9:22 AM
Addshore claimed this task.
Restricted Application added a project: User-Addshore. · View Herald TranscriptJun 10 2018, 3:49 AM