Last modified: 2014-11-17 09:21:19 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T53958, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 51958 - Watchlist and recent changes should be able to show diffs inline
Watchlist and recent changes should be able to show diffs inline
Status: NEW
Product: MediaWiki
Classification: Unclassified
Watchlist (Other open bugs)
1.22.0
All All
: Normal enhancement with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks: 51942
  Show dependency treegraph
 
Reported: 2013-07-24 12:13 UTC by Bartosz Dziewoński
Modified: 2014-11-17 09:21 UTC (History)
7 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Bartosz Dziewoński 2013-07-24 12:13:54 UTC
Watchlist and recent changes should be able to show diffs inline.

I mean, it's 2013. Seriously.
Comment 1 Jesús Martínez Novo (Ciencia Al Poder) 2013-07-26 14:39:57 UTC
Changing to "Enhancement".

It exists as a Gadget. See [[Wikipedia:Tools/Navigation popups]]
Comment 2 Rainer Rillke @commons.wikimedia 2013-08-20 14:41:04 UTC
Or at least it should allow loading the diffs using AJAX. Navigation popups is rather ugly: 1) The timeout before it sends the request 2) you can't move your pointer away or the popup closes.
Comment 3 Quiddity 2013-11-28 18:33:30 UTC
https://en.wikipedia.org/wiki/User:Writ_Keeper/Scripts/inlineDiffDocs does this.

To test:
importScript('User:Writ Keeper/Scripts/commonHistory.js');

I think this could be a good new gadget (for now, and possibly a mediawiki feature in future months/years); I shall suggest that on the talkpage, next.
Comment 4 Bartosz Dziewoński 2013-12-01 13:53:11 UTC
It doesn't seem to work with enhanced watchlist/RC :(
Comment 5 Bartosz Dziewoński 2013-12-01 13:53:45 UTC
(I definitely like what it's doing when it works, though, I was imagining pretty much exactly that.)
Comment 6 writ.keeper.enwp 2013-12-02 22:53:00 UTC
What doesn't it work with? Seems to work on my watchlist and RC on en at least; is there a different thing for which it doesn't work?
Comment 7 Quiddity 2013-12-02 23:02:16 UTC
(In reply to comment #6)
> What doesn't it work with? Seems to work on my watchlist and RC on en at
> least;
> is there a different thing for which it doesn't work?

If you "bundle" changes, (by enabling in preferences:
[Recent change]->"Group changes by page in recent changes and watchlist"
and 
[Watchlist]->"Expand watchlist to show all changes, not just the most recent")
then it doesn't work.
Comment 8 writ.keeper.enwp 2013-12-02 23:29:48 UTC
Well, jeez, it's hardly my fault that, if you switch the grouped RC changes on, it changes the format for both RC *and* watchlist to use tables instead of uls. (Is that a bug, by the way? Why would changing an RC setting also change the format of the watchlist too? Switching on the expanded watchlist without switching on the grouped RC doesn't change the format of the watchlist page, and the script still works.)  Anyway, I'll fix that.
Comment 9 writ.keeper.enwp 2013-12-03 00:37:21 UTC
Ah, nevermind, I see the point now.  Still going to try to fix it.
Comment 10 writ.keeper.enwp 2013-12-03 01:11:44 UTC
Okay, done! I think it should work on the enhanced RC and watchlist now, for both collapsing and uncollapsing entries.  Give it a shot and let me know if it works, and whether there are any more suggestions.
Comment 11 Bartosz Dziewoński 2013-12-08 17:57:41 UTC
Works and I like it :)


If we wanted to make this a core feature, I think we should just make
it hook to the "N changes" / "cur | prev" links instead of adding more
(as the watchlist interface is already rather cluttered and
intimidating).

When we do that, we should make viewing the diff inline update the
"page visitedness" status (whether the pages on Watchlist are bolded;
I think there's some default gadget at en.wp which disables this
incredibly useful feature :(, so you'd have to disable it to see the
result there).


If we were to merge your script into core MediaWiki, we'd have to make
it adhere to our coding conventions (see [[mw:CC]] and [[mw:CC/JS]])
and you'd have to agree to a relicense it under GNU GPL or a
compatible license (e.g. MIT) – unfortunately CC BY-SA (under which
you implicitly released it by putting it in a Wikipedia page) is
apparently not compatible, but things can easily have multiple
different licenses.


As for general review of the code, if you're interested :)
* After you click the [inspect diff] link, there's a period of time
  when nothing is happening (while the API request is loading) – there
  needs to be some indicator that the script is not stuck.
  I pastebinned a simple solution here: http://pastebin.com/cb7W2U9i ,
  feel free to use it.
* Why are you allowing only one API request to be active at a time?
* Selectors like ":contains" are rather slow and should not be used
  unless absolutely necessary; not sure if this is the cause, but the
  script can take a few seconds to set up the buttons on large pages
* Changing the 'type' of <input>s and <button>s after they are created
  will throw an exception on IE<=8; it's better to use something like
  $('<input type=button>')[0] to create them
* Using Object.keys will likewise throw an exception on some older
  browsers, I think.
* We have a few fancy libraries you could use instead of
  reimplementing some logic, like mediawiki.api or mediawiki.Uri, or
  the aforementioned jquery.spinner. They all are (or should be)
  documented at https://doc.wikimedia.org/mediawiki-core/master/js/
  and can be loaded using mw.loader.using([dependencies], callback)
  like in my pastebin above.
Comment 12 Rainer Rillke @commons.wikimedia 2013-12-08 18:34:49 UTC
(In reply to comment #11)
> $('<input type=button>')[0] to create them
Note that this should be valid HTML then, including closing tags for elements that need closing tags.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links