Clarify the cropTarget minting processing model #47
Conversation
index.html
Outdated
| Let |success| be a [=boolean=], initially set to false. | ||
| </li> | ||
| <li> | ||
| If [=this=] track is [=uncropped=] and |valid| is true, set |success| to true. |
There was a problem hiding this comment.
Shall we allow the UA the possibility to fail due to resource limitations?
index.html
Outdated
| </p> | ||
| </li> | ||
| <li>Let <var>p</var> be a new {{Promise}}.</li> | ||
| <li> | ||
| <p>Run the following steps in parallel:</p> | ||
| <ol> | ||
| <li> | ||
| If <var>cropTarget</var> is neither {{undefined}} nor a [=valid CropTarget=], | ||
| reject <var>p</var> with a {{NotAllowedError}} and abort these steps. | ||
| If <var>cropTarget</var> is neither {{undefined}} or if calling [=determine if |
There was a problem hiding this comment.
Is this now redundant with the steps around |success|?
(And nit: neither-nor, not neither-or, I think.)
There was a problem hiding this comment.
Dropped this indeed. Does this mean that NotAllowedError was never something that could be triggered?
There was a problem hiding this comment.
It can never trigger because cropTarget cannot be undefined. It can be null because it is nullable. NotAllowedError implies permission failure and is the wrong error here. Maybe InvalidStateError.
There are lots of problems with this spec, the biggest being it's shape lacks consensus, and we've been attacking them from big to small. FPWD is meant to be the beginning of standardization, not the end.
There was a problem hiding this comment.
I understand the current PR as making an iterative improvement that does not introduce any new non-consensus issues. If any non-consensus issues are mistakenly introduced by this PR, @jan-ivar, please flag them and we can remove them.
| <ol> | ||
| <li><p>In an [=implementation-defined=] way, propagate state associated with |cropTarget|.</p></li> | ||
| <li> | ||
| <p>If state propagation was successful, [=resolve=] |promise|. Otherwise, [=reject=] |promise|.</p> |
There was a problem hiding this comment.
This is under heavy discussion in #17 and there's no concensus to merge this. Web developers aren't going to expect this to fail, and failures are going to be surpassing and browser dependent, which is bad for web compat.
This also cements an inferior approach to this problem, which I've argued is actually slower than the alternative, a claim no one has challenged yet.
There was a problem hiding this comment.
This is under heavy discussion in #17
Please note the PR's description.
Web developers aren't going to expect this to fail
Why?
This also cements an inferior approach to this problem
https://www.w3.org/Consortium/cepc/#expected-behavior
which I've argued is actually slower than the alternative, a claim no one has challenged yet.
There was a problem hiding this comment.
#48 is where this discussion is happening. Please comment there.
|
Is the problem with this PR solely that it introduces the affordance that CropTarget-minting may fail? Or do you have other issues with it? Assuming that's the only issue - if we remove that part, and leave a note explaining that there's a debate over this issue, would that work for you, and allow consensus to emerge for this PR? |
| <ol> | ||
| <li><p>In an [=implementation-defined=] way, propagate state associated with |cropTarget|.</p></li> | ||
| <li> | ||
| <p>If state propagation was successful, [=resolve=] |promise|. Otherwise, [=reject=] |promise|.</p> |
There was a problem hiding this comment.
| <p>If state propagation was successful, [=resolve=] |promise|. Otherwise, [=reject=] |promise|.</p> | |
| <p>[=resolve=] |promise|.</p> |
| <li> | ||
| <p>If state propagation was successful, [=resolve=] |promise|. Otherwise, [=reject=] |promise|.</p> | ||
| <p class="note"> | ||
| The user agent needs to [=resolve=] or [=reject=] |promise| only after it has finished all |
There was a problem hiding this comment.
| The user agent needs to [=resolve=] or [=reject=] |promise| only after it has finished all | |
| The user agent needs to [=resolve=] |promise| only after it has finished all |
|
I'm opposed to a note. Issues are covered on github and we're discussing #48 next week. |
Fixes #48 [edit jib]. This is to help move along the discussion at #17 and to make the processing model better reflect the intended Chromium implementation.
Preview | Diff