Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed navigator.canShare. #8

Merged
merged 3 commits into from
Nov 3, 2016
Merged

Removed navigator.canShare. #8

merged 3 commits into from
Nov 3, 2016

Conversation

mgiuca
Copy link
Collaborator

@mgiuca mgiuca commented Jul 21, 2016

Resolves #5.

@mgiuca mgiuca assigned mgiuca and marcoscaceres and unassigned mgiuca Jul 21, 2016
@mgiuca
Copy link
Collaborator Author

mgiuca commented Jul 21, 2016

I'm not super convinced we don't want this (since there may be situations in which the UA sometimes has targets, sometimes does not). But best to start small and add later if necessary.

@benfredwells

@benfredwells
Copy link
Collaborator

Seems good to me, but I can't merge it :(


**TODO(mgiuca)**: This may have to be asynchronous, so that the implementation
can query the file system without blocking.
The `navigator.share` method should be `undefined` if the user agent does not
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposal here is not very idiomatic and would cause it to behave unlike any other DOM API. Share targets can be added asynchronously at any point in time (via the OS or via a share target being registered). As such, this should probably not appear and disappear from one turn of the event loop to another (or change from a function to undefined. The problem is that navigator.hasOwnProperty("share") would return true even if set to undefined ... which would be confusing).

We could work on the assumption that there is always at least 1 share target: the UA share box itself... For instance, Safari has "Add to Reading List" and "Add To Bookmarks" as persistent share targets. Other UAs might inform the user about adding share targets or might simply allow the user to cancel.

@marcoscaceres
Copy link
Member

@benfredwells, I've added you to the list of collaborators and made @mgiuca repo admin. Matt, please feel free to add additional collaborators.

@mgiuca
Copy link
Collaborator Author

mgiuca commented Jul 22, 2016

We could work on the assumption that there is always at least 1 share target: the UA share box itself... For instance, Safari has "Add to Reading List" and "Add To Bookmarks" as persistent share targets. Other UAs might inform the user about adding share targets or might simply allow the user to cancel.

Yes, we did decide on that assumption on Tuesday. (The rationale for removing canShare was to assume that there's always at least 1 share target on any supported platforms.)

I did not mean to imply that navigator.share would come and go dynamically as share targets are added and removed. If we want to support that, we should have a dedicated canShare. I just thought we had decided that you'll generally either always have at least 1 share target, or not support the API at all (on any given platform). In that case feature detection is sufficient.

I don't mind either way. I just thought we could remove canShare and add it back if we find a good reason to.

@marcoscaceres
Copy link
Member

I did not mean to imply that navigator.share would come and go dynamically as share targets are added and removed. If we want to support that, we should have a dedicated canShare. I just thought we had decided that you'll generally either always have at least 1 share target, or not support the API at all (on any given platform). In that case feature detection is sufficient.

Ah, ok. But the current phrasing is confusing then, as it makes it sounds like the browser might take the API away at arbitrary moments.

@mgiuca
Copy link
Collaborator Author

mgiuca commented Jul 25, 2016

Ah, ok. But the current phrasing is confusing then, as it makes it sounds like the browser might take the API away at arbitrary moments.

Fair enough. Reworded.

**TODO(mgiuca)**: This may have to be asynchronous, so that the implementation
can query the file system without blocking.
In a user agent that will never provide any share targets (e.g., on a platform
that does not support sharing) `navigator.share` SHOULD be `undefined`, so that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, but the problem here remains. The current reading is that the user agent would implement ".share", but it would return "undefined". This would mean that the following would return true:

var x = "share" in navigator; // true, even though it's "undefined"

Worst case, .share should be nullable - but if there is no share target at all in the platform, the user agent would not implement the API at all.

There are examples of this in the platform: for instance, Android's marketplace APK size restrictions prevent Firefox from shipping ES's Intl on Android (because it would add like 30+ megabytes to Firefox) - hence, Firefox does not ship that API on Android - and thus Intl is not implemented at all, even though it's shipped on Desktop.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, didn't realise that there's a difference between being undefined and not being defined. (bleh) Is it sufficient to say "navigator.share SHOULD NOT be present"? I'm not sure what you mean by "Worst case, .share should be nullable".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@mgiuca
Copy link
Collaborator Author

mgiuca commented Aug 24, 2016

Update: We've had some discussion on the Chrome side and it looks like it isn't great to have navigator.share be missing on some platforms but not on others. In particular, we haven't found any precedent for this: lots of mobile-only APIs (e.g. Accelerometer, Vibration, Screen Orientation) still exist on Chrome's Desktop implementations but don't do anything meaningful.

I think there's a case to be made that, in dglazkov's words: "the API surface does not change from OS to OS, but the way it responds does."

This means we probably do want canShare so we can do more targeted feature detection. It's possible we'll also want to be able to detect whether there are targets that support particular types of content, like images, so canShare would become more useful.

@PaulKinlan
Copy link

This similar thing is being discussed on the notifications spec as well whatwg/notifications#81 and whatwg/notifications#80 - for things that are not available on the platform should the notification object expose it and how should we do it.

@benfredwells
Copy link
Collaborator

benfredwells commented Sep 8, 2016

I think the navigator.share === undefined approach is attrictive when you consider other browsers. If we have canShare I can imagine this code:

if (navigator.canShare !== undefined && navigator.canShare()) {
  navigator.share(...);
}

or even, for the paranoid:

if (navigator.canShare !== undefined && navigator.canShare()) {
  if (navigator.share !== undefined) {
    navigator.share(...);
  }
}

instead of

if (navigator.share !== undefined) {
  navigator.share(...);
}

@dglazkov
Copy link

The key issue, I think, is that you end up having different varieties of a Web platform across implementations, and that seems bad for predictability. I sympathize with @benfredwells syntactic elegance argument. If there was a way to specify that share is may return undefined in WebIDL, I would be okay with that.

@dglazkov
Copy link

@RByers, to put this on Predictability folks' radar.

@mgiuca
Copy link
Collaborator Author

mgiuca commented Sep 20, 2016

@dglazkov

you end up having different varieties of a Web platform across implementations, and that seems bad for predictability

The way I see it, if you consider a method being undefined or not as a "variety", then you're always going to have two different varieties of every API until all browsers implement on all platforms, and all users update to the newer versions of those browsers:
a) undefined
b) defined

And developers always have to deal with those two "varieties".

If we add a canShare method, now there will be three varieties of the API:
a) undefined
b) defined, but canShare returns false
c) defined, and canShare returns true

In effect, forcing developers to deal with more possibilities. (I believe this is @benfredwells point.)

If there was a way to specify that share is may return undefined in WebIDL, I would be okay with that.

Do you mean a call to share() returns undefined, or that share itself is undefined? We can specify that in WebIDL but I don't think we should have to. Any API can implicitly be undefined simply by the fact that a given browser hasn't implemented it yet. If we consider Chrome Windows and Chrome Android as two different browsers, then we simply have an API that is implemented on one browser, but not on another. Which is something developers have to deal with anyway.

@dglazkov
Copy link

Thanks for explaining. I see that canShare adds, not reduces the API surface entropy. I've made peace with the idea that, on some platforms, we may be not implementing some partial interfaces (like the partial Navigator here). @RByers does that make sense to you?

@RByers
Copy link

RByers commented Oct 3, 2016

Sorry for the delay (still getting caught up after TPAC).

Thanks for explaining. I see that canShare adds, not reduces the API surface entropy. I've made peace with the idea that, on some platforms, we may be not implementing some partial interfaces (like the partial Navigator here). @RByers does that make sense to you?

Yes, in cases like this I agree it's better for developers if they have to reason only about "does this API exist or not".

Just to make sure I understand, we're talking only about variations between platforms (Android vs. Windows) here, right? Not something more dynamic? The challenge we've had in other contexts (eg. TouchEvents API, origin trials) is that it's difficult for us to make expressions like 'share' in navigator be both dynamic (something whose result can change after process startup) and efficient.

If there's no such need for dynamism here, then I agree we should definitely just leverage normal feature detection patterns around the existence of properties.

@mgiuca
Copy link
Collaborator Author

mgiuca commented Oct 4, 2016

@RByers Yes, this is (at the moment) just a static "is this supported on my platform". We would reconsider adding canShare if we needed anything dynamic (such as "are there any apps installed that handle this request").

@mgiuca
Copy link
Collaborator Author

mgiuca commented Nov 3, 2016

So we are launching this on the origin trial without canShare. It seems we have consensus on this from above. I'm going to close this out so our explainer/interface docs match what we've implemented so far.

@marcoscaceres
Copy link
Member

sounds good.

@mgiuca mgiuca merged commit 914d1bc into w3c:master Nov 3, 2016
@mgiuca mgiuca deleted the remove-canshare branch November 3, 2016 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants