Page MenuHomePhabricator

Completely remove api.js and ForeignApi.js, their functionality has been upstreamed
Closed, ResolvedPublic

Description

I think you can completely remove api.js and ForeignApi.js, I upstreamed the remaining bits of their functionality recently.

Details

Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : masterHygiene: Remove api code from MobileFrontend
mediawiki/extensions/MobileFrontend : masterHygiene: Use core code where available for api

Event Timeline

matmarex created this task.Aug 24 2015, 7:24 PM
matmarex raised the priority of this task from to Needs Triage.
matmarex updated the task description. (Show Details)
matmarex added subscribers: matmarex, Jdlrobson.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 24 2015, 7:24 PM

Change 233635 had a related patch set uploaded (by Jdlrobson):
WIP: Use core code where available for api

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

Thanks for this update matmarex. It's a little disappointing we seem to have repeated efforts on the progress events. Would be great to think about how we can do this better in future.

Unfortunately I still can't fully remove it - my patch removes everything we can though. Would be great to get this even leaner by considering whether Api should use event emitter and whether ForeignApi can support jsonp

matmarex added a comment.EditedAug 26 2015, 7:42 PM

I basically copied the code for tracking upload progress from MobileFrontend wholesale, I only changed how the actual events are emitted – as you say, we're not using OOjs EventEmitter in mediawiki.api currently, so I thought it was more consistent not to introduce it and instead use the notify/progress feature of jQuery.Deferred. Is this problematic?

I'm not sure what you mean by "support JSONP". If you want to perform non-authenticated[1] requests, you can explicitly pass centralauthtoken: false, and no token will be acquired or sent. Do you need the ability to actually use JSONP requests with a callback explicitly?

(By the way, I only just tested it and discovered that api.php?callback=foo&centralauthtoken=bar&... always fails with a badtoken error, even if the token is in fact correct. Interesting.)

[1] Or rather, a request with whatever cookies the browser happens to have for the target domain, which might correspond to a logged-in user.

You can take a look at https://gerrit.wikimedia.org/r/233635 to see what we're doing differently in MobileFrontend.
Essentially we only do a GET request via CORS when we have to to make working locally without vagrant easier and avoiding handshaking for tokens. Once that's merged I'm happy to iterate further and try and get MobileFrontend completely using core's api code.

In terms of new approach around progress that's cool - I had understood incorrectly that you guys had implemented it separately. The frontend standards group would be a great place to talk about things we are doing and identify code which extensions are using that are likely to be of use to others in future.

Jdlrobson triaged this task as High priority.Sep 2 2015, 8:04 PM
Jdlrobson set Security to None.

Change 233635 abandoned by Jdlrobson:
Hygiene: Use core code where available for api

Reason:
https://gerrit.wikimedia.org/r/237279 < new approach

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

Change 238992 had a related patch set uploaded (by Jdlrobson):
WIP: Remove api code from MobileFrontend

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

Jdlrobson claimed this task.Oct 9 2015, 8:43 PM
Jdlrobson moved this task from Backlog to Tech debt on the MobileFrontend board.Nov 4 2015, 5:17 PM

Change 238992 merged by jenkins-bot:
Hygiene: Remove api code from MobileFrontend

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

Jdlrobson closed this task as Resolved.Dec 18 2015, 8:28 PM