Add a hook to allow filtering of the target in URL before displaying the story

[Defenso-QTH]

Hello,

This discussion is a follow up of this thread on the talk.tiddlywiki.org forums: https://talk.tiddlywiki.org/t/how-can-i-prevent-tw-from-creating-a-tiddler-based-on-the-url-fragment-hash-part/9665/

I want to be able to blacklist some URL fragments (hash-parts) that are returned to my Tiddlywiki in an authentication flow.

I think the most elegant manner to let users / plugins implement this kind of features would be through a hook in $:/core/modules/startup/story.js as I propose in my post. It takes only one line and avoids having to overwrite this tiddler with a customized one, which would break on upgrades.

An alternative could be to move the invocation of th-opening-default-tiddlers-list to after we put the target in the list, so we could use that hook and avoid adding a new one:

Before

	// Process the story filter to get the story list
	var storyList = $tw.wiki.filterTiddlers(storyFilter);
	// Invoke any hooks that want to change the default story list
	storyList = $tw.hooks.invokeHook("th-opening-default-tiddlers-list",storyList);
	// If the target tiddler isn't included then splice it in at the top
	if(target && storyList.indexOf(target) === -1) {
		storyList.unshift(target);
	}

After

	// Process the story filter to get the story list
	var storyList = $tw.wiki.filterTiddlers(storyFilter);
	// If the target tiddler isn't included then splice it in at the top
	if(target && storyList.indexOf(target) === -1) {
		storyList.unshift(target);
	}
	// Invoke any hooks that want to change the default story list
	storyList = $tw.hooks.invokeHook("th-opening-default-tiddlers-list",storyList);

But it would still be added to the History by the code below, though, so a bit of a larger change might be required to make it clean.

[pmario]

I think that's the wrong place. I think what you need is a possibility to filter the locationhash before it is converted to a storyList. But I would have to have a closer look at the code first.

[pmario]

I think that's a good approach

[Defenso-QTH]

To add this into the core the code would be split into a three javascript tiddlers, a utils tiddler(with a new hook), a startup tiddler that would be invoked immediately after 'load-modules' (which would run the new utils function), both of these would be added to the core; finally a non-core javascript tiddler defining the $tw.hooks.addHook callback function (which would modify the hash).

Why would you split this in three? All I see is we either move the existing hook invocation on $:/core/modules/startup/story.js or add a new one, and have a tiddler to implement the filter with a hook (which is the equivalent of the one monkey-patching the kernel function). So there would be only one tiddler if the feature is used (and zero if not), not three.

[buggyj]

@Defenso-QTH the core has different module type, the definition of new $tw.utils.getLocationHash() with the new hook would go into a 'utils' module.

However during startup somewhere is needed to call

$tw.locationHash = $tw.utils.getLocationHash($tw.locationHash);

The core has startup modules for this, so another javascript tiddler of module type 'startup' would be required

[Defenso-QTH]

Please let me know if I do not understand your proposition correctly but you are saying we should have a hook that lets us replace the $tw.locationHash variable? That does not sound like the type of thing you would use a hook for.

When I was talking of a hook, my original idea was to take this bit of code: https://github.com/Jermolene/TiddlyWiki5/blob/master/core/modules/startup/story.js#L116-L130

function openStartupTiddlers(options) {
	options = options || {};
	// Work out the target tiddler and the story filter. "null" means "unspecified"
	var target = null,
		storyFilter = null;
	if($tw.locationHash.length > 1) {
		var hash = $tw.locationHash.substr(1),
			split = hash.indexOf(":");
		if(split === -1) {
			target = $tw.utils.decodeURIComponentSafe(hash.trim());
		} else {
			target = $tw.utils.decodeURIComponentSafe(hash.substr(0,split).trim());
			storyFilter = $tw.utils.decodeURIComponentSafe(hash.substr(split + 1).trim());
		}
	}
...

And add this line at the bottom:

	target = $tw.hooks.invokeHook("th-setting-startup-target", target);

Then you only need one tiddler to implement a hook. Like this:

/*\
title: $:/hooks/startupTargetHook.js
type: application/javascript
module-type: startup

Set up the hook to filter unwanted targets.

\*/

(function(){

/*jslint node: true, browser: true */
/*global $tw: false */
"use strict";

// Export name and synchronous status
exports.name = "startup-target-hook";
exports.synchronous = true;

// Because the hook is invoked in that script
// we can simply take advantage of the load-ordering feature.
exports.before = ["story"]; 

const auth_redirect_regex = /code=.+&state=.+&session_state=.+/;

exports.startup = function() {
	// Setup hook on th-setting-startup-target
	$tw.hooks.addHook("th-setting-startup-target", function(target) {
		if (auth_redirect_regex.test(target)) {
			return null; // If target matches the regex, we cancel it.
		} else {
			return target; // Otherwise we do not touch it.
		}
	});
}

})();

Isn't that more straightforward?

[buggyj]

@Defenso-QTH may be adding the hook into the story.js the coding would be a little easier.... however as you mentioned your proposal does not involve modifying $tw.locationHash whereas my proposal does. That is a significant difference. I am no sure which is preferable, there may be some implication for the navigation history...