Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fluff: Add [restore this revision] link to old revisions #511

Closed

Conversation

Amorymeltzer
Copy link
Collaborator

@Amorymeltzer Amorymeltzer commented Feb 2, 2019

1st commit: Tidies up and removes duplicate/unnecessary code

  • The same four lines, three of which used spanTag, were repeated frequently
  • Newer revisions either are the latest or not
  • No idea what .firstrevisionheader did but I can't find it anywhere on-wiki, so removed

2nd commit: Add [restore this revision] link to oldid pages

fluff: Add [restore this revision] link to oldid pages, closes #131

Checking for wgDiffOldId and wgDiffNewId excludes Special:Undelete, and so should be sufficient to avoid problematic diffs. Some of the logic/checks may be a tad confusing since wgDiffNewId and wgDiffOldId have different behaviors (see [[phab:T214985]]). One advantage of the restructuring is that [restore this revision] will now appear when viewing the first revision as a diff with diff=prev, as may sometimes happen.

One thing to note is that when viewing old revisions, checking for div#mw-revision-info sufficiently prevents the link from appearing on the most recent revision, since div#mw-revision-info-current is used instead. However, that is only the case if there is anything present at [[MediaWiki:Revision-info-current]], elsewise the fallback is to use div#mw-revision-info; this may have issues for other wikis if that page doesn't exist. Moreover, unlike revisionasof, $3 in revision-info doesn't expose a link we can capture, hence the imperfect Morebits.query('oldid') appears the best we can do.

modules/twinklefluff.js Outdated Show resolved Hide resolved
@MusikAnimal
Copy link
Collaborator

At cursory glance this seems to work well. I am a little concerned about checking the DOM for .mw-revision-info. What about comparing wgRevisionId and wgCurRevisionId? Could that work?

Visually, my only concern is proximity of the [restore this version] link to the breadcrumbs (say if you're viewing a subpage). I don't have any good ideas on how to address this... but I do see misclicks happening. Minor issue.

The link is pretty tiny too, but I suppose that's just a consequence of how that part of the page is styled by MediaWiki (or Common.css, etc).

- The same four lines, three of which used spanTag, were repeated frequently
- Newer revisions either are the latest or not
- No idea what .firstrevisionheader did but I can't find it anywhere on-wiki, so removed
Closes wikimedia-gadgets#131

Checking for wgDiffOldId and wgDiffNewId excludes Special:Undelete, and so
should be sufficient to avoid problematic diffs.  Some of the logic/checks may
be a tad confusing since wgDiffNewId and wgDiffOldId have different behaviors
(see [[phab:T214985]]).  One advantage of the restructuring is that [restore
this revision] will now appear when viewing the first revision as a diff with
diff=prev, as may sometimes happen.

One thing to note is that when viewing old revisions, checking for
div#mw-revision-info sufficiently prevents the link from appearing on the most
recent revision, since div#mw-revision-info-current is used instead.  However,
that is only the case if there is anything present at
[[MediaWiki:Revision-info-current]], elsewise the fallback is to use
div#mw-revision-info; this may have issues for other wikis if that page
doesn't exist.  Moreover, unlike revisionasof,
[$3 in revision-info](https://translatewiki.net/wiki/MediaWiki:Revision-info/qqq)
doesn't expose a link we can capture, hence the imperfect
Morebits.query('oldid') is the best we can do.
@Amorymeltzer
Copy link
Collaborator Author

@MusikAnimal:

What about comparing wgRevisionId and wgCurRevisionId

That was my initial plan, but ran into a few issues. The more minor one is that on deleted pages, the two are the same (0), and I was trying to simplify things by removing that. The larger issue is that wgRevisionId and wgCurRevisionId don't help distinguish diffs from old revisions, but #mw-revision-info does. It's avoidable if you test (and fail to find) #mw-diff-otitle1 and #mw-diff-otitle1, but that's just relying on a different id, so isn't much help. Using #mw-revision-info simplifies the logic quite a bit.

Visually, my only concern is proximity of the [restore this version] link to the breadcrumbs (say if you're viewing a subpage).

Ditto. I first tried putting it above the diff nav links, but I think that's even more likely to cause misclicks. I think the real issue is that, unlike diffs, this stuff isn't centered, so it all seems off-kilter and tiny next to the comparatively-giant message from #mw-revision-info. Discoverability is definitely an issue, although I hope folks are familiar enough with the bold brown text to spot it.

@Amorymeltzer
Copy link
Collaborator Author

Ignored my own advice in 8550975, so this took a while to get back to. After going through all the conflicts, I ended up refactoring this too much, so I'm closing this in favor of #561 (cosmetics) and #562 (the meat).

Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request May 10, 2019
…iff content hidden

Fixes wikimedia-gadgets#627.  If a user has "Do not show page content below diffs" selected in Special:Preferences, `wgRevisionId` will be set to 0 on diffs, meaning no links would be shown on diffs.  Comparing `wgRevisionId` to `wgDiffNewId` from wikimedia-gadgets#563 appears to be a holdover from some (local) attempts to handle merge conflicts between wikimedia-gadgets#511 and wikimedia-gadgets#407, when I initially had this structured differently, but that should now handled in the init, so this was excessive.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add [restore this version] link when viewing old revisions
2 participants