Closed Bug 1553603 Opened 6 months ago Closed 4 months ago

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

Categories

(GeckoView :: General, defect, P2)

69 Branch
Unspecified
All
defect

Tracking

(firefox-esr68 wontfix, firefox69 wontfix, firefox70 fixed)

RESOLVED FIXED
mozilla70
Tracking Status
firefox-esr68 --- wontfix
firefox69 --- wontfix
firefox70 --- fixed

People

(Reporter: tigeroakes, Assigned: droeh)

References

(Depends on 1 open bug)

Details

(Whiteboard: [geckoview:fenix:p3][wptsync upstream])

Attachments

(2 files)

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.

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

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.

Priority: -- → P2
Whiteboard: [geckoview:fenix:p3]

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?

Attachment #9076615 - Flags: feedback?(bugs)

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 on attachment 9076615 [details] [diff] [review]
Add support for capture attribute

(commented)
Attachment #9076615 - Flags: feedback?(bugs)

(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?

Flags: needinfo?(bugs)

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.

Flags: needinfo?(bugs)
Attachment #9079498 - Attachment description: Bug 1553603 - Support "capture" attribute in Gecko and expose in GV. r=smaug!,snorp!,#geckoview-reviewers! → Bug 1553603 - Support "capture" attribute in Gecko and expose in GV. r=snorp!,#geckoview-reviewers!
Attachment #9079498 - Attachment description: Bug 1553603 - Support "capture" attribute in Gecko and expose in GV. r=snorp!,#geckoview-reviewers! → Bug 1553603 - Support "capture" attribute in Gecko and expose in GV. r=snorp!,#geckoview-reviewers!,smaug!
Pushed by droeh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ec57b40d52c0
Support "capture" attribute in Gecko and expose in GV. r=geckoview-reviewers,smaug,snorp,agi
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/18180 for changes under testing/web-platform/tests
Whiteboard: [geckoview:fenix:p3] → [geckoview:fenix:p3][wptsync upstream]
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Status: UNCONFIRMED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Upstream PR merged

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.

Assignee: nobody → droeh
You need to log in before you can comment on or make changes to this bug.