flypig.co.uk

I'm continuing to try to get external content actions to work today. By that, I mean links that trigger external applications, such as "mailto" links which are supposed to open up an email for editing in an email client, "sms" links that are supposed to open a Message sending application, or "tel" links that are supposed to open the phone application.

On ESR 78 these all work flawlessly, but on ESR 91 they're currently broken. Overnight I ran a build that introduced two previous patches, patch 0056 "Use libcontentaction for custom scheme URI handling" and patch 0062 "Fix content action integration to work". These both make changes that are important for supporting these external content actions.

After installing the new packages this morning and testing the browser I find... no change. The external content links still don't work. But wait! Yesterday I was looking through the code in nsExternalHelperAppService.cpp and ContentDispatchChooser.jsm. The former calls the nsIHandlerInfo handlers while the latter implements them. During my testing I discovered that changes in ESR 91 have introduced a new check for a confirmation prompt before opening such a URL. We don't currently have a Sailfish implementation for these prompts, so the trigger is always failing, causing the handleURI() method to exit early.

After hacking around this check using the changes I made yesterday and trying again, the content action now works!

This is great news and all, and it's another moment where it feels like I'm stepping in to the light coming out of a dark tunnel. But it's not yet a solution. I still need to figure out whether the right approach is to hack around the failing prompt request, or implement it. Implementing it would be nicer for sure, but it'll really be a question of how much time and effort is involved, how big the changes are to implement it compared to working around it, and whether there are any other downsides to either approach.

So my task for today is to figure this out and implement the solution.

Looking through the code in ContentDispatchChooser.jsm it looks like there may be several ways to tackle this problem. Let's return to the code so I can explain a couple of the ways it could be done. Our end goal is to get to the point where launchWithURI() is called. As I've just established, now that the patches applied, once launchWithURI() is called everything else is now working correctly.

There are, in fact, two different ways that launchWithURI() can get called in the flow of execution. All of the following snippets of code will be taken from the handleURI() method in ContentDispatchChooser.jsm. First in this method comes this piece of logic:

    // Skip the dialog if a preferred application is set and the caller has
    // permission.
    if (
      callerHasPermission &&
      !aHandler.alwaysAskBeforeHandling &&
      (aHandler.preferredAction == Ci.nsIHandlerInfo.useHelperApp ||
        aHandler.preferredAction == Ci.nsIHandlerInfo.useSystemDefault)
    ) {
      try {
        aHandler.launchWithURI(aURI, aBrowsingContext);
      } catch (error) {
        // We are not supposed to ask, but when file not found the user most 
    likely
        // uninstalled the application which handles the uri so we will continue
        // by application chooser dialog.
        if (error.result == Cr.NS_ERROR_FILE_NOT_FOUND) {
          aHandler.alwaysAskBeforeHandling = true;
        } else {
          throw error;
        }
      }
    }

Currently this doesn't get executed because one of the disjunctive conditions that this is gated by resolves to false. A bit of testing shows that they are in fact all true apart from the callerHasPermission value. If this were set to true the entire condition would resolve to true and we'd enter the gated block, which would immediate make the required call to launchWithURK().

So one approach would be to get callerHasPermission set to true. Let's put this on our stack to return to later, so that we can cover the other options. But we will return to this value shortly.

Currently this condition is false and so nothing in the block is executed. So we move to the next part of the code which looks like this:

    let shouldOpenHandler = false;
    try {
      shouldOpenHandler = await this._prompt(
        aHandler,
        aPrincipal,
        callerHasPermission,
        aBrowsingContext
      );
    } catch (error) {
      Cu.reportError(error.message);
    }

    if (!shouldOpenHandler) {
      return;
    }

    // Site was granted permission and user chose to open application.
    // Launch the external handler.
    aHandler.launchWithURI(aURI, aBrowsingContext);

This bit of code immediately calls this._prompt() which returns a value of either true or false. If the value is false then this means the user doesn't want the external app opened and so the execution returns early. If it returns true on the other hand, the early return is skipped and execution continues right to the end of the method, including the launchWithURI() call that we're so interested in at the end.

So the process that happens inside this._prompt() is crucial for this branch of execution. The _prompt() method is pretty simple, but quite long, so rather than copy out the code here I'll just pick out sections and describe what happens instead. There are two main sections. The first checks whether the permission has been granted, as indicated by the aHasPermissions variable. If it has it does nothing. Otherwise a call is made to _openDialog() to open a window to request permissions.

The code waits for the dialog to return at which points it reads in the "granted" property to determine whether the permission was granted. If it wasn't the method returns early. If it was the value of the "resetHandlerChoice" property is stored in the resetHandlerChoice variable and the state of shouldOpenHandler is flipped from false to true.

  /**
   * Show permission or/and app chooser prompt.
   * @param {nsIHandlerInfo} aHandler - Info about protocol and handlers.
   * @param {nsIPrincipal} aPrincipal - Principal which triggered the load.
   * @param {boolean} aHasPermission - Whether the caller has permission to
   * open the protocol.
   * @param {BrowsingContext} [aBrowsingContext] - Context associated with the
   * protocol navigation.
   */
  async _prompt(aHandler, aPrincipal, aHasPermission, aBrowsingContext) {
    let shouldOpenHandler = false;
    let resetHandlerChoice = false;

    // If caller does not have permission, prompt the user.
    if (!aHasPermission) {
[...]
      await this._openDialog(
        DIALOG_URL_PERMISSION,
        {
          handler: aHandler,
          principal: aPrincipal,
          browsingContext: aBrowsingContext,
          outArgs,
          canPersistPermission,
          preferredHandlerName: this._getHandlerName(aHandler),
        },
        aBrowsingContext
      );
      if (!outArgs.getProperty("granted")) {
        // User denied request
        return false;
      }

      // Check if user wants to set a new application to handle the protocol.
      resetHandlerChoice = outArgs.getProperty("resetHandlerChoice");
[...]
      shouldOpenHandler = true;
    }

In the second segment we see the resetHandlerChoice coming up again. If it's true a dialog is opened that's intended to be used for choosing the application to use. Sailfish OS has its own mechanism for this, so if we were to implement these dialogues we'd probably want the first but not this second dialog.

In case this second dialog is opened the "openHandler" property is used to overwrite the value in shouldOpenHandler once the dialog has closed.

    // Prompt if the user needs to make a handler choice for the protocol.
    if (aHandler.alwaysAskBeforeHandling || resetHandlerChoice) {
      // User has not set a preferred application to handle this protocol 
    scheme.
      // Open the application chooser dialog
[...]
      await this._openDialog(
        DIALOG_URL_APP_CHOOSER,
        {
          handler: aHandler,
          outArgs,
          usePrivateBrowsing,
          enableButtonDelay: aHasPermission,
        },
        aBrowsingContext
      );

      shouldOpenHandler = outArgs.getProperty("openHandler");
[...]
    }

    return shouldOpenHandler;
  }

The shouldOpenHandler variable is then used as the return value for the method. That means that the value returned by _prompt() depends on how the user responds to the dialog choices presented.

We don't yet have an implementation for these dialogues, but we could consider it. But it would require quite a bit of work, because we'd need to add new functionality to the settings pages so that the values could be amended later. Given this, and since it's not functionality we currently provide, I'm going to leave this implementation for now. It would be fun to implement, but this release really doesn't need any more delay right now.

So instead I'm going to shift focus to the callerHasPermission from the early code inside the handleURI() method. The status of this variable controlled whether the prompt boxes were opened or skipped. We want the dialogues to be skipped, which means we want callerHasPermission to take the value true. So how is the value determined? By the following call:

    let callerHasPermission = this._hasProtocolHandlerPermission(
      aHandler.type,
      aPrincipal
    );

Let's take a look at the _hasProtocolHandlerPermission() method which is determining this. It can be found in the same file:

  /**
   * Test if a given principal has the open-protocol-handler permission for a
   * specific protocol.
   * @param {string} scheme - Scheme of the protocol.
   * @param {nsIPrincipal} aPrincipal - Principal to test for permission.
   * @returns {boolean} - true if permission is set, false otherwise.
   */
  _hasProtocolHandlerPermission(scheme, aPrincipal) {
    // Permission disabled by pref
    if (!nsContentDispatchChooser.isPermissionEnabled) {
      return true;
    }

    // If a handler is set to open externally by default we skip the dialog.
    if (
      Services.prefs.getBoolPref(
        "network.protocol-handler.external." + scheme,
        false
      )
    ) {
      return true;
    }

    if (!aPrincipal) {
      return false;
    }

    if (aPrincipal.isAddonOrExpandedAddonPrincipal) {
      return true;
    }

    let key = this._getSkipProtoDialogPermissionKey(scheme);
    return (
      Services.perms.testPermissionFromPrincipal(aPrincipal, key) ===
      Services.perms.ALLOW_ACTION
    );
  }

There are a few different things happening here, but right at the top of this method we see it can be short-circuited if the value of isPermissionEnabled is set to true. And that value comes from this bit of code, also in the same file:

XPCOMUtils.defineLazyPreferenceGetter(
  nsContentDispatchChooser,
  "isPermissionEnabled",
  "security.external_protocol_requires_permission",
  true
);

This is a lazy getter for the following configuration preference:

security.external_protocol_requires_permission

When this preference is set the isPermissionEnabled variable will be set also. The default value is for it to be set, so we need to change this. Toggling it manually confirms that when unset, the mailto: and sms: links now work correctly. There are quite a few options at our disposal for having this unset. One would be to have it added to the prefs.js file in the profile folder like this:

[...]
user_pref("privacy.purge_trackers.date_in_cookie_database", 
    "0");
user_pref("security.allow_disjointed_external_uri_loads", true);
user_pref("security.external_protocol_requires_permission", false);
[...]

But how to get it into this file? There are multiple options for this as well. For example it could be set in the WebEngineSettings::initialize() method where we set various other preferences. This is generally used for preferences that must be overwritten forcefully each time in order for the browser to run (so where we don't want the user to change them). But there are also preferences there which only get added to the file once when it's created. Another alternative would be to just add it to the prefs.js file that's in the sailfish-browser project. This is used as the base for the profile's prefs.js file when it's created.

The problem with both options is that they'll only take effect when the prefs.js is created for the first time. New users upgrading their browser won't the have the setting applied.

Typically we might add a one-shot for this, so that the preference gets set even when it's an upgrade, rather than a fresh install of the browser.

I could patch around the preference in the JavaScript we've just been looking at, but that would not only be ugly, it'd also mean the user couldn't then use the preference to disable external app launching.

Another option would be to flip the default value as it appears in the all.js file of gecko. This would be a convenient way not to have to worry about dealing with one-shots or upgrading preference files.

I've decided to give the last of these a go as the simplest option. It can be improved later if needed. Unfortunately this change will also require a full rebuild to test. So I've set it going and will see how it's got on in the morning.

If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.