Page MenuHomePhabricator

Have a standard way of doing hash fragment routing in MediaWiki
Closed, ResolvedPublic

Description

Multimedia viewer, MobileFrontend (and possibly other extensions) introduce a concept of routes where navigating to '#/image/<title>' executes a certain piece of JavaScript. Clicking browser back navigates away from it and URLs can be shared. Before more and more extensions write their own code for this, let's generalise this and get this into core in some form.

The MobileFrontend code is pretty flexible and OOjs compatible. I see that OOjs ui has a concept of a WindowManager that would also benefit from being able to associate hash fragment urls with certain code.

Would the OOjs library be a good place for this code to live? Could I propose we discuss this in a future frontend standardisation meeting?

https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/resources/mobile.startup/Router.js

Requirements:

  • The code should be lightweight as it's loaded early on in MobileFrontend.

Event Timeline

Jdlrobson raised the priority of this task from to Needs Triage.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added subscribers: Jdlrobson, Mooeypoo, Florian and 6 others.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 28 2015, 6:51 PM
Jdlrobson triaged this task as Normal priority.Sep 28 2015, 10:43 PM

Nothing UI- or MediaWiki-specific seems to fall within OOjs' scope.

@Jdlrobson From my perspective that fits perfectly well into the scope of frontend standards group.

Catrope edited projects, added OOUI; removed OOjs.Oct 2 2015, 5:18 AM
Catrope set Security to None.

@Catrope I actually think this is an OOjs thing... not OOjs-ui. In MobileFrontend we have a Router in our OOjs equivalent and OverlayManager in our OOjs-ui equivalent

I actually think this is an OOjs thing... not OOjs-ui.

AFAICS OOjs has been specifically kept as small and general purpose as possible.

Jdlrobson updated the task description. (Show Details)Nov 4 2015, 4:14 PM

@Catrope I actually think this is an OOjs thing... not OOjs-ui. In MobileFrontend we have a Router in our OOjs equivalent and OverlayManager in our OOjs-ui equivalent

..or neither. OOjs-ui is a UI library based on OOjs. We could have a routing library based on OOjs(-ui?).

Change 260950 had a related patch set uploaded (by Jdlrobson):
Introduce mediawiki.router for handling hash fragment navigation

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

Dropping mention of OOUI as this seems to be unrelated right now.

Tgr added a subscriber: Tgr.Feb 7 2016, 5:08 PM

cc @JGirault - I hear that you are also interested in this :)

Hey I would really like to have this feature.

I decided to use the @Jdlrobson script for a patch on Kartographer (slightly simplified for the POC), and this library made it quite easy. See https://gerrit.wikimedia.org/r/#/c/283374 and T128940.

Is it possible to move forward with this Router proposal https://gerrit.wikimedia.org/r/260950 ?

Osnard added a subscriber: Osnard.Apr 15 2016, 12:35 PM
TheDJ added a comment.Apr 15 2016, 1:30 PM

Another example of this is the tabs in Preferences.

@Jdlrobson highlighted this Phab task on wikitech-l today: "MediaWiki hash fragment Router". His message:

There is clearly a need for a generic solution to hash fragment routing in MediaWiki. So far I've seen needs in MultimediaViewer, Kartographer, MobileFrontend, Gather and potential needs in VisualEditor. There are probably other bespoke solutions in other extensions too. It would be great to take a minute and standardise on something (we can always change it later). I invite your discussion here: https://phabricator.wikimedia.org/T114007 and your thoughts on my straw man proposal: https://gerrit.wikimedia.org/r/#/c/260950/4

Back in September, Volker suggested:

In T114007#1692164 (on 2015-09-30), @Volker_E wrote:

@Jdlrobson From my perspective that fits perfectly well into the scope of frontend standards group.

That seems reasonable to me. Looking at the history of this task, it appears as though the necessary discussions may have happened as a result of that, and that Jon made a patch for this (Gerrit #260950) back in December.

Jon, is this something you're really just looking for someone to +2, or is there a wider discussion you're hoping to have first (e.g. some advice about a particular trouble spot)?

Uhm, why do we need hash fragment routing at all (in the foreseeable future)? History API is already available almost everywhere.

Tgr added a comment.Apr 15 2016, 8:26 PM

Uhm, why do we need hash fragment routing at all (in the foreseeable future)? History API is already available almost everywhere.

There are situations where you don't have a server-side fallback because the routes address something utterly JS-dependent, and using the history API would split the cache unnecessarily.

Fair point though, it would be better to have a router class that can handle non-hash-based routing as well.

Uhm, why do we need hash fragment routing at all (in the foreseeable future)? History API is already available almost everywhere.

Hi! Consider multimedia viewer. If you viewed an image in it and shared the link you wouldn't expect to share the file page would you? If you click the back button would you expect to close it or go back to the page you were in?

If you opened the mobile editor and shared the URL for the JavaScript editor it's confusing if what the user sees is the non js editor.

Many things are not documents but have a state that's useful to store and share. Hash fragments are not going anywhere IMO.

That said you are right History API could/should be used where it makes sense. We may want to consider this developing into a standard Router that handles all this. Right now the use cases I see only relate to hash fragments however - the web does a good job of the rest of our URLS :-)

I would like to add pushState support for the Router patch that @Jdlrobson did. hashchange can do most of the job, but:

  • it leaves a trailing # if you navigate to the empty route. (try location.hash = 'test'; location.hash = '')
  • I saw this code to fix the trailing hash in MultimediaViewer, but if we introduce pushState somewhere, maybe we should support it all?

My suggestion is to mimic Backbone's http://backbonejs.org/#History, because it can be considered very stable.

Backbone's Router offers many options though:

  • full pushState support
  • hashchange support
  • hashchange as a fallback to pushState.

What I suggest is to have:

  • full pushState support. Router with or without hash fragments (with fallback to hashchange?)
  • hashchange only support - do we need?
  • introduce trigger: true and replace: true options to the navigate() method : very handful in some cases.

What do you think? I can work on the patch proposal.

From the short discussion we had in the Front End Standards meeting,

  • the trailing hash is a minor issue.
  • the small MultimediaViewer hack solves that issue with pushState
  • we can almost drop hashchange in favor of pushState but IE9 is still blocking us: http://caniuse.com/#feat=history

I will make an update to @Jdlrobson's patch adding the small MultimediaViewer hack (once I verify it solves the Maps use case).

Change 260950 merged by jenkins-bot:
Introduce mediawiki.router for handling hash fragment navigation

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

Jdforrester-WMF closed this task as Resolved.May 6 2016, 3:04 PM
Jdforrester-WMF assigned this task to Jdlrobson.

OOjs-Router is now a library mastered in GOJR here, and available (wrapped as "mediawiki.router") in MediaWiki core from 1.28.0-wmf.1 onwards.

JGirault added a comment.EditedJun 17 2016, 12:17 AM

The router has a method isSupported that checks 'onhashchange' in window.

Question: Per our practices around browser support, shall we developers provide a fallback in case router.isSupported() === false ?
https://www.mediawiki.org/wiki/Compatibility
http://caniuse.com/#feat=hashchange

Example:
I have a link on the page that is : <a id="myRoute" href="#/myRoute">Click here for more details</a>.
I defined myRoute such as:

router.route( '/myRoute', showMoreDetails);

Is it fine to rely on the router only ?

Or do I have to provide a fallback? Something like:

$( '#myRoute' ).on('click', function(e) {
	e.preventDefault();
	if ( router.isSupported() ) {
		router.navigate( this.href.slice(0) );
	} else {
		showMoreDetails();
	}
});

Question: Per our practices around browser support, shall we developers provide a fallback in case router.isSupported() === false ?
https://www.mediawiki.org/wiki/Compatibility
http://caniuse.com/#feat=hashchange
Example:
I have a link on the page that is : <a id="myRoute" href="#/myRoute">Click here for more details</a>.
I defined myRoute such as:

router.route( '/myRoute', showMoreDetails);

Is it fine to rely on the router only?

Depends on the situation. If it's a link created by PHP output, then it shouldn't point to something handled by oojs-router (e.g. it should work without javascript, either point to working url, or a hash link that works natively, e.g. jumping to an element by ID - and maybe using the router for an enhanced way of opening the link).

If created dynamically as part of the interface, then relying on the router is fine since it's the router's job to make sure it works properly in all browsers in which our JavaScript run-time is enabled. Right now, oojs-router falls back from popstate to hashchange. hashchange is supported in all browsers that pass our feature test iirc.

Depends on the situation. If it's a link created by PHP output, then it shouldn't point to something handled by oojs-router (e.g. it should work without javascript, either point to working url, or a hash link that works natively, e.g. jumping to an element by ID - and maybe using the router for an enhanced way of opening the link).
If created dynamically as part of the interface, then relying on the router is fine since it's the router's job to make sure it works properly in all browsers in which our JavaScript run-time is enabled. Right now, oojs-router falls back from popstate to hashchange. hashchange is supported in all browsers that pass our feature test iirc.

Thanks @Krinkle . I fall in the second case (created and handled in JS) so I created T139554 to clean up my code.

However, the router does not use popstate. I once changed that, in a patch for T135692, but it's been stalled since we moved out of Gerrit (I will update one day but last time I tried to make the patch in the new repo I wasted 2 hours).