Fix #4: Make @capture an enumerated attribute #6
Conversation
|
See also: RawGit preview |
|
Looping in @legion80 too. |
| The <dfn data-dfn-for="CaptureFacingMode">CaptureFacingMode</dfn> | ||
| enumeration is used to express the <a>preferred facing mode</a>. The | ||
| semantics of its keywords mirror the similarly named keywords defined | ||
| in <a><code>VideoFacingModeEnum</code></a>. |
There was a problem hiding this comment.
If it's similar to VideoFacingModeEnum, why not just reuse it? I think supporting (ignoring) the left and right modes will be easier than to add another enum here.
There was a problem hiding this comment.
The VideoFacingModeEnum is video-specific, so thought adding an indirection would be justified, as it'd allow us to have control over the semantics as needed.
That said, I don't feel strongly about this, we could reuse VideoFacingMode as a hint for audio or image capture too given the enum name itself is not web-facing.
There was a problem hiding this comment.
@miguelao Let us know if the rationale for minting CaptureFacingMode enum sounds reasonable. Adding a new enum should be cheap in terms of implementation effort.
There was a problem hiding this comment.
TBH I think the naming CaptureFacingMode makes more sense than VideoFacingModeEnum, and we should just line up both specs. Go ahead with this change but we should file a bug there to rename it (there) and then bundle both, since they all describe the same concepts. (And also I wonder what's that Enum at the end of VideoFacingModeEnum doing there).
index.html
Outdated
| "CaptureFacingMode">user</dfn> and <dfn data-dfn-for= | ||
| "CaptureFacingMode">environment</dfn>, which map to the respective | ||
| states <var>user</var> and <var>environment</var>. The <a>missing | ||
| value default</a> is the <var>environment</var> state. The <a>invalid |
There was a problem hiding this comment.
Is this true? IIRC the current <input capture> just leaves it to the System App to decide what's the default orientation and we might want to maintain compatibility with it. If so, we should make it more evident/expectable than simply adding a description here, e.g. allow it to be nullable?
There was a problem hiding this comment.
Should made defaults implementation-specific indeed.
Can you expand the use case for nullable?
| <input type="submit" value="Upload"> | ||
| </form> | ||
| </pre> | ||
| </li> | ||
| <li>Or alternatively, to capture video using the device's local video | ||
| camera: | ||
| camera facing the environment: |
There was a problem hiding this comment.
Side question: is there an implementation status/ caniuse for this feature? (couldn't find any)
There was a problem hiding this comment.
https://www.w3.org/2009/dap/wiki/ImplementationStatus
Thanks for the review!
There was a problem hiding this comment.
Also it's in the queue of caniuse (Fyrd/caniuse#1115) waiting for some candid soul to add the entry (as a reference, this is the mediacapture-image).
|
LGTM |
Map 'missing value default' and 'invalid value default' to the implementation-specific state.
| The <dfn data-dfn-for="CaptureFacingMode">CaptureFacingMode</dfn> | ||
| enumeration is used to express the <a>preferred facing mode</a>. The | ||
| semantics of its keywords mirror the similarly named keywords defined | ||
| in <a><code>VideoFacingModeEnum</code></a>. |
There was a problem hiding this comment.
TBH I think the naming CaptureFacingMode makes more sense than VideoFacingModeEnum, and we should just line up both specs. Go ahead with this change but we should file a bug there to rename it (there) and then bundle both, since they all describe the same concepts. (And also I wonder what's that Enum at the end of VideoFacingModeEnum doing there).
| </p> | ||
| <p class="note"> | ||
| If the user agent is unable to support the <a>preferred facing | ||
| mode</a>, it can fall back to the implementation-specific default |
There was a problem hiding this comment.
By convention, non-normative notes contain no RFC terms (because they're non-normative).
| <input type="submit" value="Upload"> | ||
| </form> | ||
| </pre> | ||
| </li> | ||
| <li>Or alternatively, to capture video using the device's local video | ||
| camera: | ||
| camera facing the environment: |
There was a problem hiding this comment.
Also it's in the queue of caniuse (Fyrd/caniuse#1115) waiting for some candid soul to add the entry (as a reference, this is the mediacapture-image).
|
@miguelao, this attribute is still a boolean in Blink's IDL. Has this been shipped on any platform yet? If not then it'll be fine, but if it used anywhere in the wild already, this will become a problem. |
|
@foolip, for the shipping status, see whatwg/html#1102 (comment) and for the usage of the to-be-superseded boolean, see your UseCounters observations at https://bugs.chromium.org/p/chromium/issues/detail?id=240252#c19 TL:DR: the boolean usage has been low enough for everyone to be comfortable with this change. The known backwards incompatible change is whatwg/html#1102 (comment) |
|
Heh, https://codereview.chromium.org/303473004 is funny, both because I had no memory of it, and because this has already gone from enum to boolean once :) I don't mind this change, just got a bit nervous when I saw things not lining up. |
|
[@foolip you're involved in so many things it is no wonder you did not remember. Luckily Google never forgets ;-)] Now we're moving ahead with the implementation of the enum at https://crbug.com/698853 as you noticed already. |
|
FWIW, this change has landed in trunk WebKit. |
|
@hober, are these the right WebKit bugs to track? |
There has been sufficient interest among implementers to beef up the
capturehint, so here's the first stab at it.Related issues:
#4
whatwg/html#1102
PTAL @miguelao @riju @hober