1553603 - Support the capture attribute on <input type="file"> in GeckoView

Status: RESOLVED FIXED

(GeckoView :: IME, defect, P2)

Description

[Tiger Oakes]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:69.0) Gecko/20100101 Firefox/69.0

Steps to reproduce:

Tap on a "Choose files" button corresponding to <input type="file" accept="image/*" capture="user">

Actual results:

The capture attribute is ignored

Expected results:

The capture attribute should open the Android device's front facing camera to take a photo that will be uploaded.

Comment 1

[Tiger Oakes]

• Related bug for Core: https://bugzilla.mozilla.org/show_bug.cgi?id=741393
• Spec (CR): http://www.w3.org/TR/html-media-capture/
• Test suite (approved): http://w3c-test.org/html-media-capture/

I'm working on the implementation for the prompt on the Android Components side, but GeckoView needs to update its file prompt call.

Comment 2

[Chris Peterson [:cpeterson]]

This would require some core Gecko work to handle the new "user" and "environment" capture flags. This would be a good intern bug.

GV also has problems with some other <input type="file"> types we could fix at the same time.

Comment 3

[Dylan Roeh (:droeh) (he/him)]

Attachment: Add support for capture attribute

I'm working on this at the moment and have something that seems to work, but hits a web platform test failure I don't quite understand: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=254950220&repo=try&lineNumber=2660

smaug, do you have any advice here?

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

That test is apparently bogus. You can test with other properties on HTMLInputElement interface, and if you try to access the property
on the prototype, it throws, also in other browsers.
Commented on https://github.com/web-platform-tests/wpt/commit/126495b573f4778fff1a2f6919ada92f794e0c2e

Based on https://wpt.fyi/results/custom-elements/reactions/HTMLInputElement.html?label=master&label=experimental no browser runs that test.

https://heycam.github.io/webidl/#dfn-attribute-getter

Want to fix the test? Perhaps
"capture" in HTMLInputElement.prototype

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9076615 [details] [diff] [review]
Add support for capture attribute

(commented)

Comment 6

[Dylan Roeh (:droeh) (he/him)]

(In reply to Olli Pettay [:smaug] from comment #4)

That test is apparently bogus. You can test with other properties on HTMLInputElement interface, and if you try to access the property
on the prototype, it throws, also in other browsers.
Commented on https://github.com/web-platform-tests/wpt/commit/126495b573f4778fff1a2f6919ada92f794e0c2e

Based on https://wpt.fyi/results/custom-elements/reactions/HTMLInputElement.html?label=master&label=experimental no browser runs that test.

https://heycam.github.io/webidl/#dfn-attribute-getter

Want to fix the test? Perhaps
"capture" in HTMLInputElement.prototype

Thanks. Couple of followups:
Wouldn't it be better to just expect a failure on this test until the issue is fixed upstream? (I assume we don't want to modify web platform tests locally -- if I'm wrong about that I'll just include a fix for the test in the patch.)

When trying to run with a fix for the test I'm running into a couple of failures:

TEST-UNEXPECTED-FAIL | /custom-elements/reactions/HTMLInputElement.html | capture on HTMLInputElement must enqueue an attributeChanged reaction when adding new attribute - assert_equals: expected (string) "" but got (object) null
TEST-UNEXPECTED-FAIL | /custom-elements/reactions/HTMLInputElement.html | capture on HTMLInputElement must enqueue an attributeChanged reaction when adding invalid value default - assert_equals: expected (string) "" but got (object) null

Essentially it seems like capture should be getting set to an empty string rather than null when unspecified -- is there a way of specifying a default value or something like that that I'm missing?

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

The test should be fixed upstream real soon https://github.com/web-platform-tests/wpt/pull/17781
(or apparently one part of the test)

About the other case, the spec is super vague here. https://w3c.github.io/html-media-capture/
"In addition, there is a third state, the implementation-specific state. "
"The missing value default is the implementation-specific state. The invalid value default is also the implementation-specific state. "

But, the test looks wrong again.
assert_attribute_log_entry(logEntries.last(), {name: 'capture', oldValue: '', newValue: 'user', namespace: null});
The old value should be null, since there was no "capture" content attribute when instance['capture'] = 'user'; was run.
Or am I missing something - at least I don't see "capture" being set anywhere before instance['capture'] = 'user';

And 'capture on HTMLInputElement must enqueue an attributeChanged reaction when adding invalid value default' is also wrong, as far as I see, because of the same issue as above.

Comment 8

[Dylan Roeh (:droeh) (he/him)]

Attachment: Bug 1553603 - Support "capture" attribute in Gecko and expose in GV. r=snorp!,#geckoview-reviewers!,smaug!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ec57b40d52c0
Support "capture" attribute in Gecko and expose in GV. r=geckoview-reviewers,smaug,snorp,agi

Comment 10

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/18180 for changes under testing/web-platform/tests

Comment 11

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 12

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/ec57b40d52c0

Comment 13

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged

Comment 14

[Chris Peterson [:cpeterson]]

Assigning to Dylan since he fixed this bug.

firefox69=wontfix because we don't need to uplift to GV 69 Beta. The capture attribute can ride the trains with GV 70.

Comment 15

[Chris Peterson [:cpeterson]]

Moving some input bugs to the new GeckoView::IME component.