canShare() is not future compatible

[marcoscaceres]

I wonder if we can simplify canShare() a bit... right now, canShare() conflates type checking behavior by intertwining the method call with share(). This is problematic because share() returns a promise, not TypeErrors.

Over on Mozilla’s bugzilla, @ericwilligers mentioned that the intent of canShare() is to avoid having to do UA sniffing. As such, why don’t we this instead:

enum Shareable {
  "file”,
  "url”,
   // and so on...
}

partial interface Navigator {
   boolean canShare(sequence<Shareable> members);
}

That greatly simplifies things by deferring the support checking to the IDL layer. That causes the IDL layer to throw if passed some unsupported Shareable type. The result being that it either returns true or throws.

If throwing is not palatable, then an alternative would be to take a sequence of DOMStrings, then return false if the names don’t match a supported member type.

[raymeskhoury]

I like that rough idea because I can see that often sites would want to check this up front to determine whether to enable file sharing.

Currently we also restrict the mime types and extensions that can be shared to a limited set. canShare could also be useful to determine if the site can share a particular type.

So perhaps canShare -> canShareFileTypes([array of file extensions/mime types]).

I'm interested in @mgiuca's thoughts here.

[marcoscaceres]

This is somewhat reminiscent of the defunct navigator.registerContentHandler() ... obviously, this is different as we want to check mine type support... however, dealing with mime types gets extremely complicated very quickly; like, sharing video, but it's in some weird compression format. One share target may support a file type, but another might not (even if the UA supports the type).

We might need to restrict the types... or maybe look at how other parts of the platform deals with this? For example, drag and drop must somehow deal with this somehow. And probably other parts of the platform too.... I'm looking at HTML Media Capture, and the interop story there also seems to be not great: https://caniuse.com/#feat=html-media-capture

Hmmm... 🤔

[ewilligers]

canShare() conflates type checking behavior by intertwining the method call with share(). This is problematic because share() returns a promise, not TypeErrors.

We simply wanted to avoid specifying the same list of conditions in two different places, to avoid the risk of them ever getting out of sync. I see the following as equivalent:-

1. Listing the conditions in share(), and saying canShare() returns false iff share() would reject with TypeError.

2. Listing the conditions in canShare(), and saying share() rejects with TypeError iff canShare would return false.

3. Defining an algorithm elsewhere, and referencing it from canShare() and share().

The spec can easily be reworded using 2. or 3. instead of 1. It is certainly not anticipated that canShare() would be implemented by calling share().

As I noted in the Mozilla bug, canShare [currently] reveals no information about which file contents/names/MIME types are accepted for sharing on a given device.

An implementation might choose to perform asynchronous malware checking in share(), and reject with NotAllowedError if the content is considered dangerous. Such checks are currently not relevant for canShare, which is synchronous.

[marcoscaceres]

We simply wanted to avoid specifying the same list of conditions in two different places, to avoid the risk of them ever getting out of sync.

Right, but I'm arguing that they are completely different concerns: they do completely different things (so there is zero risk of them getting out of sync as there is no overlap in functionality):

• canShare() = "Does the browser support sharing any of [URL / File / text]? or will any of those cause part of my share() to be dropped on the floor?" (For example, a L1 implementation, would drop file on the floor given something like share({ url: validURL, file: file, ...}).

• share() = "asynchronously try to share this thing... which may have type errors, invalid URLs, and other weird things".

It is certainly not anticipated that canShare() would be implemented by calling share().

That's not my reading. The spec pretty clearly states that the two are co-dependent (my emphasis): "canShare(data) MUST return true unless share(data) would reject with TypeError, in which case it MUST return false."

Clearly, to me, "share(data) would reject with TypeError" moves the risk of things getting out of sync onto implementers. That seems unfortunate and unnecessary.

As I noted in the Mozilla bug, canShare [currently] reveals no information about which file contents/names/MIME types are accepted for sharing on a given device.

Which I think is great and a good point that you raised it again. The above proposal prevents us from being tempted to go down the MIME checking route entirely: it only says yea or nay to supporting particular dictionary members.

An implementation might choose to perform asynchronous malware checking in share(), and reject with NotAllowedError if the content is considered dangerous.

If we need a malware checker to implement file, I don't think I will get this past our security team. That's a non-starter.

I wonder if we can put in place some provision to only allow some well-known, inert, file types that we all agree to?... probably not... but maybe worth considering?

Such checks are currently not relevant for canShare, which is synchronous.

Agree. But again, this is why I think there should be no overlap between what the two methods do.

[ewilligers]

If we need a malware checker to implement file, I don't think I will get this past our security team. That's a non-starter.

Not required, the spec just doesn't prevent it.

I wonder if we can put in place some provision to only allow some well-known, inert, file types that we all agree to?.

Blink currently supports the following:

Audio

                "flac"  -  audio/flac
                "m4a"  -  audio/x-m4a
                "mp3"  -  audio/mp3
                "oga"  -  audio/ogg
                "ogg"  -  audio/ogg
                "opus"  -  audio/ogg
                "wav"  -  audio/wav
                "weba"  -  audio/webm

Image

                "bmp"  -  image/bmp
                "gif"  -  image/gif
                "ico"  -  image/x-icon
                "jfif"  -  image/jpeg
                "jpeg"  -  image/jpeg
                "jpg"  -  image/jpeg
                "pjp"  -  image/jpeg
                "pjpeg"  -  image/jpeg
                "png"  -  image/png
                "svg"  -  image/svg+xml
                "svgz"  -  image/svg+xml
                "tif"  -  image/tiff
                "tiff"  -  image/tiff
                "webp"  -  image/webp
                "xbm" - image/x-xbitmap

Text

                "css"  -  text/css
                "csv"  -  text/csv
                "ehtml"  -  text/html
                "htm"  -  text/html
                "html"  -  text/html
                "shtm"  -  text/html
                "shtml"  -  text/html
                "text"  -  text/plain
                "txt"  -  text/plain

Video

                "m4v"  -  video/mp4
                "mp4"  -  video/mp4
                "mpeg"  -  video/mpeg
                "mpg"  -  video/mpeg
                "ogm"  -  video/ogg
                "ogv"  -  video/ogg
                "webm"  -  video/webm

[marcoscaceres]

@ewilligers @mgiuca, I'd like to pick this up again and see if we can simplify the design a bit... I'd like to continue investigating the possibility of only checking on for the member names. We can probably drop the enum and just use a DOMString, i.e.

navigator.canShare(["files", "url"])

Which works nice with:

navigator.canShare(Object.keys(data));

Alternative is to make it variadic, which might give some flexibility for single checks without needing to create an array.

// check all...
navigator.canShare(...Object.keys(data));
// check some... 
const supported = memberNames.filter(name => navigator.canShare(name));

[inexorabletash]

I'm a bit confused by the intent of this issue. Is this just calling for improving how canShare() and share() are specified, or is it about altering what they do w/r/t file type support?

Assuming the former: I agree, the spec text could be improved.

I'd just factor out the algorithm from share() so both methods can "call" it, e.g.

To validate share data with data and base, run the following steps:

1. If none of data's members title, text or url are present:
   1. If data's member files is not present or is empty, or if the implementation does not support file sharing, return false.
2. If data's url member is present:
   1. Let url be the result of running the URL parser on data's url, with base, and no encoding override.
   2. If url is failure, return false.
3. Return true.

Then it can be called with:

1. Let base be this's relevant settings object's API base URL.
2. If running the steps to validate share data with data and base return false, then return a promise rejected with a TypeError.

(share() would need to reparse the url, but can Assert that it's not failure)

[othermaciej]

I agree this would be a simplification. If the app already has a ShareData, then the current canShare may be convenient. But effectively, the API is only useful to check whether file sharing is supported. It seems likely web apps would want to check that well before they get to the point of creating a ShareData. In such cases, I'd expect a web app to omit file sharing UI, or to offer download UI instead, based on a single up-front check. I seems inconvenient to make a fake ShareData to do that.

One complication is that, if content currently feature-detects and uses canShare, a different method with the same name would be a compatibility risk. It might be good to give the method a different name for this reason (e.g. canShareType or the even narrower canShareFiles).

Accepting either form of the argument is not an ideal solution, because there'd be no way to feature test which form of arguments is accepted.

[ewilligers]

But effectively, the API is only useful to check whether file sharing is supported.

The idea was that in future there might be more fields, and there should be a way to feature detect if they are supported.

navigator.canShare(["files", "url"]) achieves the objective more directly.

[marcoscaceres]

navigator.canShare(["files", "url"]) achieves the objective more directly.

This is true, but you still don't know which one is not supported? Is it files or is it url? so you are back to .canShare(["files"]) || canShare(["whatever"]).

What @othermaciej proposes deals with this problem more elegantly: it allows you to feature detect if you support files for the purpose of feature detection, by virtue that the method name says what it queries: "canShareFiles()".

[ewilligers]

I have no concerns about adding canShareFiles. A web app might want to know if the user agent will allow sharing of PDF files, for example, or if the user agent would preemptively block them regardless of any apps that might be installed.

Blink would likely retain support for canShare:

The original PR introducing canShare was uploaded in September 2018 and reviewed in October 2018.

Blink's Intent to Ship for file sharing was in January 2019.

The feature detection recommendation Google shared with web developers in June 2019 was

if (navigator.canShare && navigator.canShare({ files: filesArray })) 

[marcoscaceres]

The original PR introducing canShare was uploaded in September 2018 and reviewed in October 2018.

Ok, respectfully, yes the PR was upload and reviewed - but it was by two folks on the same team in the same company. Please understand why that is concerning form a standardization perspective: features should receive wide review and endorsement/support from multiple implementers. We have the PR template for a reason.

Same with the other things that were mentioned. The stuff Google did there is great (the intent to ship, the feature recommendation on the Chrome dev site, etc.), but those are very Chrome/Google specific things.

[othermaciej]

I think there are two separate questions here:

1. Is there enough deployed content that's calling .canShare to feature test for file sharing that it's worthwhile for the spec to support it for compatibility? This is a question for standards. If Google has any data on this it would be helpful.
2. If the answer to (1) is "no", then should Chrome continue to ship a nonstandard extension to the eventual standard? I think that is up to the Chrome team.

Hopefully we don't end up with different answers. (2) is one of the reasons it's risky to ship draft standards before getting sufficient input from the group and from other vendors, as @marcoscaceres said.

[tomayac]

1. Is there enough deployed content that's calling .canShare to feature test for file sharing that it's worthwhile for the spec to support it for compatibility? This is a question for standards. If Google has any data on this it would be helpful.

According to ChromeStatus data the canShare() API is used on ~0.0083% of page loads.

[marcoscaceres]

So, it seems I might have left this one too late to change. Safari 14 ships with the checks (though I think File is still pref'ed off).

https://github.com/WebKit/webkit/blob/master/Source/WebCore/page/Navigator.cpp#L127

We should move .canShare() to the main spec, but based on @inexorabletash proposed changes: #108 (comment).

[marcoscaceres]

@whsieh, @othermaciej, @ericwilligers, @mgiuca, during review of Mozilla's implementation, @saschanaz has found a fundamental flaw with the API that makes this not future compatible.

Imagine tomorrow we add images member to ShareData:

const someObj = { url: "someURL", images: [] }; 
navigator.canShare(someObj); // true! Oh no!

Today, all implementations would return true to the above, because "images" is dropped on the floor by IDL layer. That means that they API is only usable if a developer destructs an object and tests it using it component parts.

Additionally, file testing is also somewhat flawed (as are the rest of the members), because they presuppose a developer knows what they are going to share before sharing it.

Again, for example, to check if files sharing is supported, one can't do:

navigator.share({files: []}); // false, at least one file needed - but might confuse people into thinking it's not supported

// so developers need to create fake empty files
navigator.share({files: [new File([], "name")]}); // maybe true

I understand that the API has shipped, but again, the API demonstrably flawed and will could lead to confusion. I'd strongly urge us to:

1. come up with a simpler design that doesn't conflate error checking with feature support (see canShare() is not future compatible  #108 (comment)).
2. Deprecate .canShare() or allow it to just take an enum value.
3. Or add new new method entirely.

Thoughts?

[ewilligers]

The simplest approach might be to add canShareFiles(), accepting an array of MIME types or file extensions.

I started to spec it, but would appreciate feedback on the approach before I refine it.

[marcoscaceres]

@whsieh, @othermaciej, @ericwilligers, @mgiuca, we are kinda at an impasse here with Mozilla (@saschanaz) rightfully pointing out the current design is broken:

#108 (comment)

Is there a willingness to fix or update .canShare() or ship something different/better? Or should we just call it "good enough" and add something new down the road (and just land pull request #177).

[saschanaz]

One way to fix that is to change the signature to canShare(optional object data = {}), check if there is any unsupported field, and then convert it to a dictionary. This way there is no need to introduce another API.

[marcoscaceres]

Yeah, that's not overly offensive - using object is a bit 🤢, but doing the conversion then gets everything back into a good state.

[saschanaz]

If no one responds, I think it's okay to proceed with object since the change would be good-first-bug-level straightforward.

[marcoscaceres]

We would need to still need to send patches and get implementer agreement, but you are right that it should be trivial. Worst case, we patch Gecko only, which gives a compatibility path forward for now (and update other implementations when this becomes an actual issue in the future).

[marcoscaceres]

Reopened so we can keep bouncing ideas here.