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

FF supports navigator.share behind pref #12194

Merged
merged 11 commits into from
Oct 7, 2021

Conversation

hamishwillee
Copy link
Collaborator

@hamishwillee hamishwillee commented Aug 27, 2021

Navigator.share() was marked false for FireFox desktop. However testing shows that it does work for desktop Windows (at least). I think it was added in FF71 in this bug behind the preference dom.webshare.enabled.
Note that FF android is not blocked by this preference.

I also did some testing and verified that Navigator.share() and Navigator.canShare() work on Windows Opera and Edge, and also Opera Android. I have guessed the versions for desktop match Chrome (but without the partial implementation warning due, I think, to MacOS issues).

Upshot, those changes might perhaps revert to true if you think this is not a safe assumption.

FYI these methods are protected by web-share permission. I have not added subfeatures for that, because I don't know if it is a compatibility issue (at all).

@github-actions github-actions bot added the data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Aug 27, 2021
api/Navigator.json Outdated Show resolved Hide resolved
@hamishwillee
Copy link
Collaborator Author

@ddbeck Further to this, the webshare spec states that navigator.share() should support sharing a base set of data types that might be shared, which includes: title, text, url, an array files

I just found this bug https://bugzilla.mozilla.org/show_bug.cgi?id=1635700 which indicates that FF does not support file sharing.

Should we note this by marking the support in firefox as partial OR by adding a subfeature for files, url, whatever. Thoughts?

@saschanaz Do you know when/if this feature is expected for Firefox?

@ddbeck ddbeck self-requested a review September 6, 2021 13:43
@ddbeck
Copy link
Collaborator

ddbeck commented Sep 8, 2021

but without the partial implementation warning due, I think, to MacOS issues

Sorry, I didn't quite follow this. You didn't mark the Mac partial implementation because you didn't test it? Or had reason to believe that this is a Chrome-specific limitation? The rest of this seems fine to me, I just want to figure out when/where we ought to be copying notes.

@hamishwillee
Copy link
Collaborator Author

but without the partial implementation warning due, I think, to MacOS issues

Coming back to this after a couple of weeks - yes, that note is gibberish sorry.

What I am saying is that Chrome marks canShare() support as partial - should I copy this "partial" to the other chromiums?

The only reason I am questioning this is because I don't know the reason this was marked as partial support in the first place. If this could be because it is not supported on macOS then I could omit this caveat on android platforms.

I guess I should add "partial" and the note for Opera though, as that runs on Mac.

Does the question make sense now?

@ddbeck
Copy link
Collaborator

ddbeck commented Sep 10, 2021

Does the question make sense now?

Yes, this makes sense now. Reading the linked bug, it appears to be Mac specific, so I'd copy it to Edge and Opera. 👍

@hamishwillee
Copy link
Collaborator Author

Thanks @ddbeck ,
I've added the note and partial_implementation to opera and Edge. Should be good now.

@saschanaz
Copy link
Contributor

I just found this bug https://bugzilla.mozilla.org/show_bug.cgi?id=1635700 which indicates that FF does not support file sharing.

Should we note this by marking the support in firefox as partial OR by adding a subfeature for files, url, whatever. Thoughts?

@saschanaz Do you know when/if this feature is expected for Firefox?

It's not expected anytime soon, and files support is added later to the spec separately, so 👍 to add a separate files data.

@saschanaz
Copy link
Contributor

And also probably a good chance to use preview since it's enabled only on Nightly/early beta.

@hamishwillee
Copy link
Collaborator Author

hamishwillee commented Sep 13, 2021

@ddbeck @saschanaz OK, I added a files subfeature with versions based on

Question. Could/should we add subfeatures for text as well? The reason I ask is that Firefox for android does not support text sharing (at the moment captured with a note).

And also probably a good chance to use preview since it's enabled only on Nightly/early beta.

@ddbeck How do you do both a preference and a preview version? This is what it looks like now

            "firefox": {
              "version_added": "71",
              "flags": [
                {
                  "type": "preference",
                  "name": "#dom.webshare.enabled",
                  "value_to_set": "enabled"
                }
              ]
            },

This was added in nightly in FF92 - see https://bugzilla.mozilla.org/show_bug.cgi?id=1641280 . I think it was only added for Windows though - dunno if that matters (i.e. it doesn't if preview is assumed to be Windows for Firefox).
I tried a few variants like this, but it can't cope with "preview". Ideas?

            "firefox": [
              {
                "version_added": "preview"
              },
              {
                "version_added": "71",
                "version_removed": "91",
                "flags": [
                  {
                    "type": "preference",
                    "name": "#dom.webshare.enabled",
                    "value_to_set": "enabled"
                  }
                ]
              }
            ],

Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

Thanks, Hamish. Some general commentary here and a specific suggestion in a line comment.

Could/should we add subfeatures for text as well? The reason I ask is that Firefox for android does not support text sharing (at the moment captured with a note).

Yes, I endorse this. 👍

This was added in nightly in FF92 - see https://bugzilla.mozilla.org/show_bug.cgi?id=1641280 . I think it was only added for Windows though - dunno if that matters (i.e. it doesn't if preview is assumed to be Windows for Firefox).
I tried a few variants like this, but it can't cope with "preview". Ideas?

There's a lot to unpack here:

  • Operating system limitations imply partial_implementation. "preview" shouldn't change this.

  • You said 'it can't cope with "preview"'. Sorry if I'm being dense, but can you elaborate on this? What's "it" and what is not coping, in this situation?

  • Based on the example, I'd interpret that to mean:

    • In Nightly-only, it's enabled by default.
    • In 71-91, it's available behind a pref.
    • In 92+ (stable), it's no longer supported in anyway.

    Is that the story you want to tell? If it isn't, what is?

api/Navigator.json Outdated Show resolved Hide resolved
@hamishwillee
Copy link
Collaborator Author

Thanks, all helpful. I'll make the required changes tomorrow.

@ddbeck

  1. By "can't cope with preview", I mean I tried the following and got an error related to using the term "preview". I tried a number of other variants, and they gave me an error warning too. So my question was "what does it have to look like". I will try this again tomorrow and post the actual error for this to provide more data.
            "firefox": [
              {
                "version_added": "preview"
              },
              {
                "version_added": "71",
                "version_removed": "91",
                "flags": [
                  {
                    "type": "preference",
                    "name": "#dom.webshare.enabled",
                    "value_to_set": "enabled"
                  }
                ]
              }
            ],
  1. With respect to the story ...

The story that I "know" is that prior to FF92 it is available behind pref ( 71-91), in FF92 it is available in Nightly by default and behind the pref for release.

The restriction to Windows confuses me. My original understanding was that the preference change in Nightly was only for Windows, but that doesn't make much sense.

@saschanaz is this a partial implementation? - ie this only works on Windows, or is it that we only enabled it for Windows, or is it that it behaves differently on Windows? Can you confirm?

@saschanaz
Copy link
Contributor

saschanaz commented Sep 16, 2021

@saschanaz is this a partial implementation? - ie this only works on Windows, or is it that we only enabled it for Windows, or is it that it behaves differently on Windows? Can you confirm?

It's enabled on Windows and Android, and should generally work same way on both, except a nit: Windows does not support tracking share action properly, so it resolves after opening the share popup, not after completing the share action. (Same for Chrome)

@hamishwillee
Copy link
Collaborator Author

hamishwillee commented Sep 17, 2021

except a nit: Windows does not support tracking share action properly, so it resolves after opening the share popup, not after completing the share action. (Same for Chrome)

@saschanaz Ah, that wasn't clear to me, and makes this wrong: https://developer.mozilla.org/en-US/docs/Web/API/Navigator/share
image

My understanding was that is that this should be re-written as:

On Windows the method resolves a Promise with undefined as soon as the share popup has launched. On other platforms it resolves the promise with undefined once data is successfully passed to the share target.

Is that correct? Note the assertions:

  • in both cases it resolves with undefined.
  • resolved when "data is passed to the share target" (i.e. bluetooth) NOT when actually sent/recieved. So completion on Android means "prompt has closed and data sent to the OS API that does the sending"

I will correct the docs in line with your comment, but NOT record this nit as a compatibility issue in BCD.

@hamishwillee
Copy link
Collaborator Author

@ddbeck I have added data_parameter_files and data_parameter_text to share and canShare. These mirror each other. The Chrome mac bug refers to files and it isn't clear if it also applies to text - but it might so I have left that in.

With respect to adding preview, I still don't get how to do that. The story I am trying to tell is that it was added in version 71 behind a preference. From version 92 it is released (no pref required) in Nightly.

I tried the following, but get Error: Invalid argument not valid semver ('preview' received)

            "firefox": [
              {
                "version_added": "preview"
              },
              {
                "version_added": "71",
                "flags": [
                  {
                    "type": "preference",
                    "name": "#dom.webshare.enabled",
                    "value_to_set": "enabled"
                  }
                ]
              }
            ],

@saschanaz
Copy link
Contributor

resolved when "data is passed to the share target" (i.e. bluetooth) NOT when actually sent/recieved.

I don't think this can be a strong assertion as each OS may behave different. But correct at least on Android.

@hamishwillee
Copy link
Collaborator Author

@ddbeck When you get a chance, this needs re-review. As discussed, this is not an immediate priority, just keeping it visible.

Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

Thanks, Hamish. Some suggestions in line comments.

Regarding Error: Invalid argument not valid semver ('preview' received), that's a bug. I'll open an issue about it.

api/Navigator.json Show resolved Hide resolved
api/Navigator.json Show resolved Hide resolved
api/Navigator.json Show resolved Hide resolved
api/Navigator.json Show resolved Hide resolved
api/Navigator.json Show resolved Hide resolved
api/Navigator.json Show resolved Hide resolved
api/Navigator.json Show resolved Hide resolved
api/Navigator.json Show resolved Hide resolved
api/Navigator.json Show resolved Hide resolved
},
"firefox_android": {
"version_added": "79",
"partial_implementation": true,
"notes": "Firefox for Android does not support sharing files or text."
"notes": "Firefox for Android does not support sharing text or files."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we have the subfeatures now, we can drop this note entirely

hamishwillee and others added 3 commits October 4, 2021 10:34
Co-authored-by: Daniel D. Beck <daniel@ddbeck.com>
Co-authored-by: Daniel D. Beck <daniel@ddbeck.com>
@hamishwillee
Copy link
Collaborator Author

@ddbeck Thanks for the review. I've "accepted all".

Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

Thank you! 🎉

@ddbeck ddbeck merged commit 2611b28 into mdn:main Oct 7, 2021
@hamishwillee hamishwillee deleted the navigator_webshare branch October 7, 2021 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants