1483552 - Async Clipboard: readText should not be exposed to the web content (limit it only to WebExtensions)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Masataka Yakura [:myakura]]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:63.0) Gecko/20100101 Firefox/63.0
Build ID: 20180813100104

Steps to reproduce:

1. open console
2. type in `'readText' in navigator.clipboard` and check out it returns true
3. go to https://myakura.github.io/samples/async-clipboard/readtext-1.html
4. copy some text to the clipboard, then hit the "Paste" button


Actual results:

paste should succeed.


Expected results:

paste fails, and we don't have any clue.

I saw on the Intent thread that readText() is enabled only in WebExtensions.
https://groups.google.com/d/msg/mozilla.dev.platform/ef7O7PrmNh0/qy8k9HDjCQAJ

I couldn't find any source whether it is true or not. However, given that paste fails, I suspect so.

Then please don't expose it in the web content. Do so only in WebExtensions.
Making features available-but-always-fail doesn't sound a great idea.

I wrote the following in the demo code.

```
// disable button if we don't have that functionality
if (!('clipboard' in navigator && 'readText' in navigator.clipboard)) {
  pasteButton.disabled = true;
  pasteButton.title("Use Ctrl/Cmd+V to paste manually.");
}
```

In Firefox, the button isn't became disabled despite the fact readText() always fails.
It just confuses people. Please please make it invisible unless you intend to make it work on the web as well.

Comment 1

[Anny Gakhokidze [:annyG]]

Attachment: Bug 1483552 - Async Clipboard: readText should not be exposed to the web content, r=nika

Comment 2

[Olli Pettay [:smaug][[e-mail]](vacation until Nov 16)]

[Tracking Requested - why for this release]:
Shouldn't this have been fixed for 63?

Comment 3

[Pascal Chevrel:pascalc]

(In reply to Olli Pettay [:smaug] from comment #2)
> [Tracking Requested - why for this release]:
> Shouldn't this have been fixed for 63?

Do you consider that bug important enough to uplift a patch fixing this bug in a 63 dot release if we have one?

Comment 4

[Olli Pettay [:smaug][[e-mail]](vacation until Nov 16)]

That is really a question for nika.
Are we going to ship broken API?

Comment 5

[Pascal Chevrel:pascalc]

How about removing the paragraph announcing that we support this API in https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/63#APIs as a short term mitigation? (we ship 63 in less than 2 hours)

Comment 6

[Nika Layzell [:nika] (ni? for response)]

(In reply to Olli Pettay [:smaug] from comment #4)
> That is really a question for nika.
> Are we going to ship broken API?

I'm not sure this is important enough to do a dot-release on. The bug here is that we expose a function to content JS which will unconditionally fail when called, and the worst-case scenario here is a program using feature-detection will try to call it to trigger a permission prompt, which we won't show.

Chromium also exposes this API to content JS but provides a permission prompt to actually allow access which we do not.

Potentially if we get a patch here we could uplift it, especially if there are webcompat issues (which I doubt right now).

I'll whip together a minimal patch just in case.

Comment 7

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1483552 - Disable Clipboard.readText outside of system/extension code,

Comment 8

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1483552 - Switch BackstagePass to use WebIDLGlobalNameHash,

Comment 9

[Pascal Chevrel:pascalc]

Tracking for potential web compat regressions in 63.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/34aacac44c01
Disable Clipboard.readText outside of system/extension code, r=baku

Comment 11

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 9019881 [details]
Bug 1483552 - Disable Clipboard.readText outside of system/extension code,

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1479935

User impact if declined: Clipboard.readText API exposed to web content which will always resolve with a rejection.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Minor change to enabled condition.

String changes made/needed: None.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9019881 [details]
Bug 1483552 - Disable Clipboard.readText outside of system/extension code,

[Triage Comment]
Approved for 64.0b5.

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e6e5c054a7f8

Comment 14

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/34aacac44c01

Comment 15

[Pascal Chevrel:pascalc]

Comment on attachment 9019881 [details]
Bug 1483552 - Disable Clipboard.readText outside of system/extension code,

Given that the impact to end-users seems minimal if any and that we are a month away from 64 which has the patch, I am not taking the patch into our next 63 dot release. Thanks!