FIX: Include subfolder base_uri in web app manifest shortcuts (PR #10878)

GitHub

I’m not sure this should be a URL

Yeah I thought the same, since the key is action, not url. However:

https://w3c.github.io/web-share-target/

When a share takes place, if the user selects this share target, the user agent opens a new browsing context at the action URL, with query parameter values containing the shared data, just like an HTML form submission.

Hmm, but the example here is a path:

"share_target": {
  "action": "/share-target/",
  "method": "GET",
  "params": {
    "title": "title",
    "text": "text",
    "url": "url"
  }
}

Oh I think I understand the question now. Discourse.base_uri is not a full URL. For most sites it is an empty string. For subfolders, it will be something like /forum

So adding this will not result in a URL, it will remain a path.

Ohhh, I should have known. Looks good to me then :+1:

Updated to use the base_path alias, which is much clearer. @danielwaterworth would you mind giving me another :+1:

Do want start_url to be a relative URL? I see it supports those (https://developer.mozilla.org/en-US/docs/Web/Manifest/start_url), but since “the manifest URL is used as the base URL to resolve it” I’m not 100% sure the final URL is correct. Doesn’t the “manifest URL” already contain the base_path? :thinking:

This isn’t a functional change, base_uri and base_path are aliases

For a subfolder, it will never be a relative URL. base_path includes a / at the beginning for subfolder sites, so I think it will be an absolute URL.

Although, reading that, it sounds like we could probably get away with '.' even for subfolder sites… But I am inclined not to change it - the current implementation of start_url is working fine. The diff here is just changing base_uri for the less confusing base_path

I understand that. :slightly_smiling_face: My questions are: was it correct in the first place, and if it was, isn’t it better to be explicit and use Discourse.base_url? Even just for the sake of people looking at the code and thinking “a path in a *_url field?” :wink:

Oops, it didn’t load your comment for me, @davidtaylorhq. :eyes: