Last modified: 2014-11-19 19:59:13 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 T57371, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 55371 - API action=query&prop=revisions doesn't return diff in xml
API action=query&prop=revisions doesn't return diff in xml
Status: NEW
Product: MediaWiki
Classification: Unclassified
API (Other open bugs)
unspecified
All All
: High major (vote)
: ---
Assigned To: Nobody - You can work on this!
:
: 66054 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-06 15:57 UTC by Peter Bena
Modified: 2014-11-19 19:59 UTC (History)
7 users (show)

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


Attachments

Description Peter Bena 2013-10-06 15:57:23 UTC
When I execute this api:

:action=query&prop=revisions&format=json&rvprop=content&rvlimit=1&rvtoken=rollback&rvdiffto=prev&titles=User%3APetrb

it return content and diff

but if I execute xml:

:action=query&prop=revisions&format=xml&rvprop=content&rvlimit=1&rvtoken=rollback&rvdiffto=prev&titles=User%3APetrb

I only get a content of page.

This makes it necessary to execute 2 different queries in order to get same thing in xml which you can have from any other format
Comment 1 Peter Bena 2013-10-06 15:57:51 UTC
this was tested on english wiki
Comment 2 Brad Jorsch 2013-10-07 14:41:48 UTC
Yet another bug with the XML format. Basically, it can't handle having both a "text content" (i.e. an item with key '*', from ApiResult::setContent()) and non-scalar sub-items at the same time. There's supposed to be an error thrown in this situation, but the test at ApiFormatXml.php line 166 is broken so it won't actually get thrown except in certain limited cases.

I can see three possible solutions here:
1. Use a key such as 'content' instead of '*'. But that would break BC for all clients, even those not trying to use diff and content at the same time, so I don't much like that idea.
2. Have the XML formatter shove '*' into a particular named subelement when other subelements are needed. Whether this subelement's name should be hardcoded or if we should allow/require something like setIndexedTagName() I'm not sure.
3. WONTFIX it.

I don't like #1 because it would break BC for all clients, even those not trying to use diff and content at the same time. #2 feels like a hack, but it's the better option if we don't want to WONTFIX this.
Comment 3 Brad Jorsch 2013-10-07 15:20:21 UTC
FYI: I don't see any other situation in core that would be affected by this.

As for WMF-deployed extensions, CodeReview's list=coderevisions has the issue if 'message' is requested along with 'tags', 'followups', or 'followedup'. I don't see any other extensions that look like they'd be affected.
Comment 4 Peter Bena 2013-10-07 15:40:21 UTC
I don't really see what is so hard on fixing this, you can easily combine

<rev ...>content</rev>

and

<diff>content of diff</diff>

what's the problem?
Comment 5 Peter Bena 2013-10-07 15:41:45 UTC
what's the point of whole star thingie? Why not just name every property with some solid name instead of some weird characters? It wouldn't look so hackish but it would surely work just as fine...
Comment 6 Brad Jorsch 2013-10-07 15:55:44 UTC
(In reply to comment #4)
> I don't really see what is so hard on fixing this, you can easily combine
> 
> <rev ...>content</rev>
> 
> and
> 
> <diff>content of diff</diff>
> 
> what's the problem?

How do you combine <rev>text...</rev> and <rev><diff>...</diff></rev>? <rev>text...<diff>...</diff></rev> or <rev><diff>...</diff>text...</rev> makes no sense in XML.

My option 1 is changing it to <rev content="text..."/>, so you could combine them easily as <rev content="text..."><diff>...</diff></rev>. My option 2 is doing basically the same (or <rev><something>text...</something><diff>...</diff></rev>) only when the XML formatter runs into this situation.


(In reply to comment #5)
> what's the point of whole star thingie? Why not just name every property with
> some solid name instead of some weird characters? It wouldn't look so hackish
> but it would surely work just as fine...

The "star thingie" is how you get <rev>text...</rev> instead of <rev content="text..."/> in the XML format. And while it would probably have been a good idea to avoid it back in 2006, changing it now would break huge numbers of existing clients which is not something we'd like to do.
Comment 7 Peter Bena 2013-10-08 09:46:36 UTC
But you could still make some other way how to ask for a result which would look like this, so that it wouldn't break any tools that expect the current format.

In fact, even if it did

<rev>text</rev>

in case you don't ask for a diff

and

<rev content="text">
  <diff>...</diff>
</rev>

in case you do ask for a diff

it would still hardly break anything, since as you said, the later should have thrown exception (which it doesn't because of some bug as you said: "There's supposed to be an error thrown
in this situation, but the test at ApiFormatXml.php line 166 is broken so it
won't actually get thrown except in certain limited cases.")

so if it produced a different kind of output instead of exception, how could it break anything?
Comment 8 Brad Jorsch 2014-06-02 22:18:09 UTC
*** Bug 66054 has been marked as a duplicate of this bug. ***
Comment 9 Bill Traynor 2014-11-19 15:01:25 UTC
As indicated on https://www.mediawiki.org/wiki/API:Main_page#The_format there are plans to remove all output formats except for JSON, should this Bug be moved to a WON'T FIX status?
Comment 10 Brad Jorsch 2014-11-19 16:39:52 UTC
No. Chances are I'm going to do something along the lines of option 2 as part of https://www.mediawiki.org/wiki/API/Architecture_work/Planning#Changes_to_XML_output_format
Comment 11 Bill Traynor 2014-11-19 18:33:52 UTC
Is the statement about moving to JSON only output formats incorrect, or just far in the future?
Comment 12 Brad Jorsch 2014-11-19 19:59:13 UTC
Far in the future, if we ever get to it. But killing formats other than json, xml, and php is in progress now.

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


Navigation
Links