Page MenuHomePhabricator

Watch star on Timeless is not AJAX-ified
Closed, ResolvedPublic

Description

Watch star on Timeless is not AJAX-ified - clicking it sends you to a separate page, instead of watching/unwatching and showing a popup.

Event Timeline

matmarex created this task.Aug 13 2017, 9:38 PM
matmarex added a subscriber: Krinkle.

This is actually a core feature that doesn't for with Timeless - see resources/src/mediawiki/page/watch.js.

Specifically this seems to be a problem:

		var $links = $( '.mw-watchlink a, a.mw-watchlink' );
		// Restrict to core interfaces, ignore user-generated content
		$links = $links.filter( ':not( #bodyContent *, #content * )' );

The mw-watchlist class is present, but in Timeless almost the entire page is wrapped in a <div id="content">, so the second check ignores the links.

Possibly this selector should be changed to #mw-body-content or something?

If the whole point of that is to ignore user provided content can we set and look for data-mw-watch attributes that are blacklisted from wikitext?

Hmm, yeah... that'll require some extra effort to make sure it works with all the skins, but it seems like a better long-term solution.

Ltrlg awarded a token.Aug 14 2017, 8:27 AM
Isarra added a subscriber: Isarra.Aug 15 2017, 2:27 PM

Well, we could just move where the content wrapper encompasses, except
that all IS the page content, so perhaps not the best idea. If excluding
user-provided content is the idea, though, shouldn't there be a class
specifically for that? Or is that what mw-body-content is for? What if
you have multiple separated blocks of content?

Legoktm claimed this task.Aug 15 2017, 8:30 PM

Change 372076 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] Support AJAX watch in skins that use a different class structure

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

Well, we could just move where the content wrapper encompasses, except
that all IS the page content, so perhaps not the best idea. If excluding
user-provided content is the idea, though, shouldn't there be a class
specifically for that? Or is that what mw-body-content is for? What if
you have multiple separated blocks of content?

We recently introduced the mw-parser-output class (T37247), but currently it is not present around all user-generated content (e.g. not page status indicators). Using it would probably still be a better solution than the current code (it didn't exist when that was written), but I think Legoktm's idea is even better than that.

Makes sense.

matmarex closed this task as Resolved.Aug 17 2017, 7:40 PM
matmarex removed a project: Patch-For-Review.
matmarex added subscribers: Mooeypoo, Bawolff.

By the way, the two other features that the patch relies on also didn't exist when the original code was written:

Change 372076 merged by jenkins-bot:
[mediawiki/core@master] Support AJAX watch in skins that use a different class structure

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