I think 446 comments is enough
More than enough. Let us be rid of it! I reviewed this again quickly and although there are very minor nits I could pick, I'd rather see this in core, unblocking bigger and better things.
Merged into 1.x with Lanny's approval.
What if my filenames were carefully crafted?
Then you shouldn't trust core with it. Core munges filenames (e.g., appending a number to a file if it already exists with the same name), and there are modules that munge filenames even further.
If you feel very strongly about a filename being "just so", uploading it into Drupal is not a good idea.
I discussed this briefly with @alexpott and the real problem here is that core allows file URIs to be invalid as URIs. I'm pretty sure URIs are not allowed to contain spaces (and certain other characters) that aren't URL-encoded.
I'm not sure exactly what the right fix here is, but Canvas adding the validation that the URIs are actually URIs is what's breaking this, and probably rightfully so.
@xjm requested in Slack that this be tagged as an 11.3.0 release priority.
@xjm requested in Slack that this be tagged as an 11.3.0 release priority.
*raises hand* Will be at NEDCamp. Have code in Canvas. 😈
Auto-merged into 8.x-2.x. Thanks everyone!
phenaproxima → made their first commit to this issue’s fork.
Thanks everyone. Auto-merged into 8.x-2.x!
The problem is being caused by this exception:
The website encountered an unexpected error. Try again later.<br><br><em class="placeholder">Drupal\Component\Serialization\Exception\InvalidDataTypeException</em>: A colon cannot be used in an unquoted mapping value at line 22 (near "Lyco, oratione locuples, rebus ipsis ielunior. Duo Reges: constructio interrete. Sed haec in pueris; Sed utrum hortandus es nobis, Luci, inquit, an etiam tua sponte propensus es? Sapiens autem semper beatus est et est aliquando in dolore; Immo videri fortasse. Paulum, cum regem Persem captum adduceret, eodem flumine invectio? Et ille ridens: Video, inquit, quid agas;&#039;"). in <em class="placeholder">Drupal\Component\Serialization\Yaml::decode()</em> (line <em class="placeholder">43</em> of <em class="placeholder">core/lib/Drupal/Component/Serialization/Yaml.php</em>). <pre class="backtrace">Symfony\Component\Yaml\Parser->doParse('Lyco, oratione locuples, rebus ipsis ielunior. Duo Reges: constructio interrete. Sed haec in pueris; Sed utrum hortandus es nobis, Luci, inquit, an etiam tua sponte propensus es? Sapiens autem semper beatus est et est aliquando in dolore; Immo videri fortasse. Paulum, cum regem Persem captum adduceret, eodem flumine invectio? Et ille ridens: Video, inquit, quid agas;&#039;', 514) (Line: 531)
Symfony\Component\Yaml\Parser->parseBlock(21, 'Lyco, oratione locuples, rebus ipsis ielunior. Duo Reges: constructio interrete. Sed haec in pueris; Sed utrum hortandus es nobis, Luci, inquit, an etiam tua sponte propensus es? Sapiens autem semper beatus est et est aliquando in dolore; Immo videri fortasse. Paulum, cum regem Persem captum adduceret, eodem flumine invectio? Et ille ridens: Video, inquit, quid agas;&#039;', 514) (Line: 190)
Symfony\Component\Yaml\Parser->doParse('- &#039;Lorem ipsum dolor sit amet, consectetur adipiscing elit. Negat esse eam, inquit, propter se expetendam. Primum Theophrasti, Strato, physicum se voluit; Id mihi magnum videtur. Itaque mihi non satis videmini considerare quod iter sit naturae quaeque progressio. Quare hoc videndum est, possitne nobis hoc ratio philosophorum dare. Est enim tanti philosophi tamque nobilis audacter sua decreta defendere.&#039;
- &#039;Huius, Lyco, oratione locuples, rebus ipsis ielunior. Duo Reges: constructio interrete. Sed haec in pueris; Sed utrum hortandus es nobis, Luci, inquit, an etiam tua sponte propensus es? Sapiens autem semper beatus est et est aliquando in dolore; Immo videri fortasse. Paulum, cum regem Persem captum adduceret, eodem flumine invectio? Et ille ridens: Video, inquit, quid agas;&#039;
- &#039;Quae cum dixisset, finem ille. Quamquam non negatis nos intellegere quid sit voluptas, sed quid ille dicat. Progredientibus autem aetatibus sensim tardeve potius quasi nosmet ipsos cognoscimus. Gloriosa ostentatio in constituendo summo bono. Qui-vere falsone, quaerere mittimus-dicitur oculis se privasse; Duarum enim vitarum nobis erunt instituta capienda. Comprehensum, quod cognitum non habet? Qui enim existimabit posse se miserum esse beatus non erit. Causa autem fuit huc veniendi ut quosdam hinc libros promerem. Nunc omni virtuti vitium contrario nomine opponitur.&#039;', 514) (Line: 531)
Symfony\Component\Yaml\Parser->parseBlock(20, '- &#039;Lorem ipsum dolor sit amet, consectetur adipiscing elit. Negat esse eam, inquit, propter se expetendam. Primum Theophrasti, Strato, physicum se voluit; Id mihi magnum videtur. Itaque mihi non satis videmini considerare quod iter sit naturae quaeque progressio. Quare hoc videndum est, possitne nobis hoc ratio philosophorum dare. Est enim tanti philosophi tamque nobilis audacter sua decreta defendere.&#039;
- &#039;Huius, Lyco, oratione locuples, rebus ipsis ielunior. Duo Reges: constructio interrete. Sed haec in pueris; Sed utrum hortandus es nobis, Luci, inquit, an etiam tua sponte propensus es? Sapiens autem semper beatus est et est aliquando in dolore; Immo videri fortasse. Paulum, cum regem Persem captum adduceret, eodem flumine invectio? Et ille ridens: Video, inquit, quid agas;&#039;
- &#039;Quae cum dixisset, finem ille. Quamquam non negatis nos intellegere quid sit voluptas, sed quid ille dicat. Progredientibus autem aetatibus sensim tardeve potius quasi nosmet ipsos cognoscimus. Gloriosa ostentatio in constituendo summo bono. Qui-vere falsone, quaerere mittimus-dicitur oculis se privasse; Duarum enim vitarum nobis erunt instituta capienda. Comprehensum, quod cognitum non habet? Qui enim existimabit posse se miserum esse beatus non erit. Causa autem fuit huc veniendi ut quosdam hinc libros promerem. Nunc omni virtuti vitium contrario nomine opponitur.&#039;', 514) (Line: 318)
Symfony\Component\Yaml\Parser->doParse(Array, 514) (Line: 86)
Symfony\Component\Yaml\Parser->parse('checkbox:
- true
color:
- &#039;#ffffcc&#039;
- &#039;#ffffcc&#039;
- &#039;#ccffff&#039;
email:
- &#039;example@example.com&#039;
- &#039;test@test.com&#039;
- &#039;random@random.com&#039;
language_select:
- en
machine_name:
- &#039;loremipsum&#039;
- &#039;oratione&#039;
- &#039;dixisset&#039;
tel:
- &#039;123-456-7890&#039;
- &#039;098-765-4321&#039;
textarea:
- &#039;Lorem ipsum dolor sit amet, consectetur adipiscing elit. Negat esse eam, inquit, propter se expetendam. Primum Theophrasti, Strato, physicum se voluit; Id mihi magnum videtur. Itaque mihi non satis videmini considerare quod iter sit naturae quaeque progressio. Quare hoc videndum est, possitne nobis hoc ratio philosophorum dare. Est enim tanti philosophi tamque nobilis audacter sua decreta defendere.&#039;
- &#039;Huius, Lyco, oratione locuples, rebus ipsis ielunior. Duo Reges: constructio interrete. Sed haec in pueris; Sed utrum hortandus es nobis, Luci, inquit, an etiam tua sponte propensus es? Sapiens autem semper beatus est et est aliquando in dolore; Immo videri fortasse. Paulum, cum regem Persem captum adduceret, eodem flumine invectio? Et ille ridens: Video, inquit, quid agas;&#039;
- &#039;Quae cum dixisset, finem ille. Quamquam non negatis nos intellegere quid sit voluptas, sed quid ille dicat. Progredientibus autem aetatibus sensim tardeve potius quasi nosmet ipsos cognoscimus. Gloriosa ostentatio in constituendo summo bono. Qui-vere falsone, quaerere mittimus-dicitur oculis se privasse; Duarum enim vitarum nobis erunt instituta capienda. Comprehensum, quod cognitum non habet? Qui enim existimabit posse se miserum esse beatus non erit. Causa autem fuit huc veniendi ut quosdam hinc libros promerem. Nunc omni virtuti vitium contrario nomine opponitur.&#039;
text_format:
- value: &#039;&lt;p&gt;Lorem ipsum dolor sit amet, consectetur adipiscing elit. Negat esse eam, inquit, propter se expetendam. Primum Theophrasti, Strato, physicum se voluit; Id mihi magnum videtur. Itaque mihi non satis videmini considerare quod iter sit naturae quaeque progressio. Quare hoc videndum est, possitne nobis hoc ratio philosophorum dare. Est enim tanti philosophi tamque nobilis audacter sua decreta defendere.&lt;/p&gt;&#039;
- value: &#039;&lt;p&gt;Huius, Lyco, oratione locuples, rebus ipsis ielunior. Duo Reges: constructio interrete. Sed haec in pueris; Sed utrum hortandus es nobis, Luci, inquit, an etiam tua sponte propensus es? Sapiens autem semper beatus est et est aliquando in dolore; Immo videri fortasse. Paulum, cum regem Persem captum adduceret, eodem flumine invectio? Et ille ridens: Video, inquit, quid agas;&lt;/p&gt;&#039;
- value: &#039;&lt;p&gt;Quae cum dixisset, finem ille. Quamquam non negatis nos intellegere quid sit voluptas, sed quid ille dicat. Progredientibus autem aetatibus sensim tardeve potius quasi nosmet ipsos cognoscimus. Gloriosa ostentatio in constituendo summo bono. Qui-vere falsone, quaerere mittimus-dicitur oculis se privasse; Duarum enim vitarum nobis erunt instituta capienda. Comprehensum, quod cognitum non habet? Qui enim existimabit posse se miserum esse beatus non erit. Causa autem fuit huc veniendi ut quosdam hinc libros promerem. Nunc omni virtuti vitium contrario nomine opponitur.&lt;/p&gt;&#039;
url:
- &#039;http://example.com&#039;
- &#039;http://test.com&#039;
webform_email_confirm:
- &#039;example@example.com&#039;
- &#039;test@test.com&#039;
- &#039;random@random.com&#039;
webform_email_multiple:
- &#039;example@example.com, test@test.com, random@random.com&#039;
webform_time:
- &#039;09:00&#039;
- &#039;17:00&#039;
', 514) (Line: 37)
Drupal\Component\Serialization\Yaml::decode('checkbox:
- true
color:
- &#039;#ffffcc&#039;
- &#039;#ffffcc&#039;
- &#039;#ccffff&#039;
email:
- &#039;example@example.com&#039;
- &#039;test@test.com&#039;
- &#039;random@random.com&#039;
language_select:
- en
machine_name:
- &#039;loremipsum&#039;
- &#039;oratione&#039;
- &#039;dixisset&#039;
tel:
- &#039;123-456-7890&#039;
- &#039;098-765-4321&#039;
textarea:
- &#039;Lorem ipsum dolor sit amet, consectetur adipiscing elit. Negat esse eam, inquit, propter se expetendam. Primum Theophrasti, Strato, physicum se voluit; Id mihi magnum videtur. Itaque mihi non satis videmini considerare quod iter sit naturae quaeque progressio. Quare hoc videndum est, possitne nobis hoc ratio philosophorum dare. Est enim tanti philosophi tamque nobilis audacter sua decreta defendere.&#039;
- &#039;Huius, Lyco, oratione locuples, rebus ipsis ielunior. Duo Reges: constructio interrete. Sed haec in pueris; Sed utrum hortandus es nobis, Luci, inquit, an etiam tua sponte propensus es? Sapiens autem semper beatus est et est aliquando in dolore; Immo videri fortasse. Paulum, cum regem Persem captum adduceret, eodem flumine invectio? Et ille ridens: Video, inquit, quid agas;&#039;
- &#039;Quae cum dixisset, finem ille. Quamquam non negatis nos intellegere quid sit voluptas, sed quid ille dicat. Progredientibus autem aetatibus sensim tardeve potius quasi nosmet ipsos cognoscimus. Gloriosa ostentatio in constituendo summo bono. Qui-vere falsone, quaerere mittimus-dicitur oculis se privasse; Duarum enim vitarum nobis erunt instituta capienda. Comprehensum, quod cognitum non habet? Qui enim existimabit posse se miserum esse beatus non erit. Causa autem fuit huc veniendi ut quosdam hinc libros promerem. Nunc omni virtuti vitium contrario nomine opponitur.&#039;
text_format:
- value: &#039;&lt;p&gt;Lorem ipsum dolor sit amet, consectetur adipiscing elit. Negat esse eam, inquit, propter se expetendam. Primum Theophrasti, Strato, physicum se voluit; Id mihi magnum videtur. Itaque mihi non satis videmini considerare quod iter sit naturae quaeque progressio. Quare hoc videndum est, possitne nobis hoc ratio philosophorum dare. Est enim tanti philosophi tamque nobilis audacter sua decreta defendere.&lt;/p&gt;&#039;
- value: &#039;&lt;p&gt;Huius, Lyco, oratione locuples, rebus ipsis ielunior. Duo Reges: constructio interrete. Sed haec in pueris; Sed utrum hortandus es nobis, Luci, inquit, an etiam tua sponte propensus es? Sapiens autem semper beatus est et est aliquando in dolore; Immo videri fortasse. Paulum, cum regem Persem captum adduceret, eodem flumine invectio? Et ille ridens: Video, inquit, quid agas;&lt;/p&gt;&#039;
- value: &#039;&lt;p&gt;Quae cum dixisset, finem ille. Quamquam non negatis nos intellegere quid sit voluptas, sed quid ille dicat. Progredientibus autem aetatibus sensim tardeve potius quasi nosmet ipsos cognoscimus. Gloriosa ostentatio in constituendo summo bono. Qui-vere falsone, quaerere mittimus-dicitur oculis se privasse; Duarum enim vitarum nobis erunt instituta capienda. Comprehensum, quod cognitum non habet? Qui enim existimabit posse se miserum esse beatus non erit. Causa autem fuit huc veniendi ut quosdam hinc libros promerem. Nunc omni virtuti vitium contrario nomine opponitur.&lt;/p&gt;&#039;
url:
- &#039;http://example.com&#039;
- &#039;http://test.com&#039;
webform_email_confirm:
- &#039;example@example.com&#039;
- &#039;test@test.com&#039;
- &#039;random@random.com&#039;
webform_email_multiple:
- &#039;example@example.com, test@test.com, random@random.com&#039;
webform_time:
- &#039;09:00&#039;
- &#039;17:00&#039;
') (Line: 43)
Drupal\webform\Utility\WebformYaml::decode('checkbox:
- true
color:
- &#039;#ffffcc&#039;
- &#039;#ffffcc&#039;
- &#039;#ccffff&#039;
email:
- &#039;example@example.com&#039;
- &#039;test@test.com&#039;
- &#039;random@random.com&#039;
language_select:
- en
machine_name:
- &#039;loremipsum&#039;
- &#039;oratione&#039;
- &#039;dixisset&#039;
tel:
- &#039;123-456-7890&#039;
- &#039;098-765-4321&#039;
textarea:
- &#039;Lorem ipsum dolor sit amet, consectetur adipiscing elit. Negat esse eam, inquit, propter se expetendam. Primum Theophrasti, Strato, physicum se voluit; Id mihi magnum videtur. Itaque mihi non satis videmini considerare quod iter sit naturae quaeque progressio. Quare hoc videndum est, possitne nobis hoc ratio philosophorum dare. Est enim tanti philosophi tamque nobilis audacter sua decreta defendere.&#039;
- &#039;Huius, Lyco, oratione locuples, rebus ipsis ielunior. Duo Reges: constructio interrete. Sed haec in pueris; Sed utrum hortandus es nobis, Luci, inquit, an etiam tua sponte propensus es? Sapiens autem semper beatus est et est aliquando in dolore; Immo videri fortasse. Paulum, cum regem Persem captum adduceret, eodem flumine invectio? Et ille ridens: Video, inquit, quid agas;&#039;
- &#039;Quae cum dixisset, finem ille. Quamquam non negatis nos intellegere quid sit voluptas, sed quid ille dicat. Progredientibus autem aetatibus sensim tardeve potius quasi nosmet ipsos cognoscimus. Gloriosa ostentatio in constituendo summo bono. Qui-vere falsone, quaerere mittimus-dicitur oculis se privasse; Duarum enim vitarum nobis erunt instituta capienda. Comprehensum, quod cognitum non habet? Qui enim existimabit posse se miserum esse beatus non erit. Causa autem fuit huc veniendi ut quosdam hinc libros promerem. Nunc omni virtuti vitium contrario nomine opponitur.&#039;
text_format:
- value: &#039;&lt;p&gt;Lorem ipsum dolor sit amet, consectetur adipiscing elit. Negat esse eam, inquit, propter se expetendam. Primum Theophrasti, Strato, physicum se voluit; Id mihi magnum videtur. Itaque mihi non satis videmini considerare quod iter sit naturae quaeque progressio. Quare hoc videndum est, possitne nobis hoc ratio philosophorum dare. Est enim tanti philosophi tamque nobilis audacter sua decreta defendere.&lt;/p&gt;&#039;
- value: &#039;&lt;p&gt;Huius, Lyco, oratione locuples, rebus ipsis ielunior. Duo Reges: constructio interrete. Sed haec in pueris; Sed utrum hortandus es nobis, Luci, inquit, an etiam tua sponte propensus es? Sapiens autem semper beatus est et est aliquando in dolore; Immo videri fortasse. Paulum, cum regem Persem captum adduceret, eodem flumine invectio? Et ille ridens: Video, inquit, quid agas;&lt;/p&gt;&#039;
- value: &#039;&lt;p&gt;Quae cum dixisset, finem ille. Quamquam non negatis nos intellegere quid sit voluptas, sed quid ille dicat. Progredientibus autem aetatibus sensim tardeve potius quasi nosmet ipsos cognoscimus. Gloriosa ostentatio in constituendo summo bono. Qui-vere falsone, quaerere mittimus-dicitur oculis se privasse; Duarum enim vitarum nobis erunt instituta capienda. Comprehensum, quod cognitum non habet? Qui enim existimabit posse se miserum esse beatus non erit. Causa autem fuit huc veniendi ut quosdam hinc libros promerem. Nunc omni virtuti vitium contrario nomine opponitur.&lt;/p&gt;&#039;
url:
- &#039;http://example.com&#039;
- &#039;http://test.com&#039;
webform_email_confirm:
- &#039;example@example.com&#039;
- &#039;test@test.com&#039;
- &#039;random@random.com&#039;
webform_email_multiple:
- &#039;example@example.com, test@test.com, random@random.com&#039;
webform_time:
- &#039;09:00&#039;
- &#039;17:00&#039;
') (Line: 96)
Drupal\webform\Utility\WebformYaml::tidy('checkbox:
- true
color:
- &#039;#ffffcc&#039;
- &#039;#ffffcc&#039;
- &#039;#ccffff&#039;
email:
- &#039;example@example.com&#039;
- &#039;test@test.com&#039;
- &#039;random@random.com&#039;
language_select:
- en
machine_name:
- &#039;loremipsum&#039;
- &#039;oratione&#039;
- &#039;dixisset&#039;
tel:
- &#039;123-456-7890&#039;
- &#039;098-765-4321&#039;
textarea:
- &#039;Lorem ipsum dolor sit amet, consectetur adipiscing elit. Negat esse eam, inquit, propter se expetendam. Primum Theophrasti, Strato, physicum se voluit; Id mihi magnum videtur. Itaque mihi non satis videmini considerare quod iter sit naturae quaeque progressio. Quare hoc videndum est, possitne nobis hoc ratio philosophorum dare. Est enim tanti philosophi tamque nobilis audacter sua decreta defendere.&#039;
- &#039;Huius, Lyco, oratione locuples, rebus ipsis ielunior. Duo Reges: constructio interrete. Sed haec in pueris; Sed utrum hortandus es nobis, Luci, inquit, an etiam tua sponte propensus es? Sapiens autem semper beatus est et est aliquando in dolore; Immo videri fortasse. Paulum, cum regem Persem captum adduceret, eodem flumine invectio? Et ille ridens: Video, inquit, quid agas;&#039;
- &#039;Quae cum dixisset, finem ille. Quamquam non negatis nos intellegere quid sit voluptas, sed quid ille dicat. Progredientibus autem aetatibus sensim tardeve potius quasi nosmet ipsos cognoscimus. Gloriosa ostentatio in constituendo summo bono. Qui-vere falsone, quaerere mittimus-dicitur oculis se privasse; Duarum enim vitarum nobis erunt instituta capienda. Comprehensum, quod cognitum non habet? Qui enim existimabit posse se miserum esse beatus non erit. Causa autem fuit huc veniendi ut quosdam hinc libros promerem. Nunc omni virtuti vitium contrario nomine opponitur.&#039;
text_format:
- value: &#039;&lt;p&gt;Lorem ipsum dolor sit amet, consectetur adipiscing elit. Negat esse eam, inquit, propter se expetendam. Primum Theophrasti, Strato, physicum se voluit; Id mihi magnum videtur. Itaque mihi non satis videmini considerare quod iter sit naturae quaeque progressio. Quare hoc videndum est, possitne nobis hoc ratio philosophorum dare. Est enim tanti philosophi tamque nobilis audacter sua decreta defendere.&lt;/p&gt;&#039;
- value: &#039;&lt;p&gt;Huius, Lyco, oratione locuples, rebus ipsis ielunior. Duo Reges: constructio interrete. Sed haec in pueris; Sed utrum hortandus es nobis, Luci, inquit, an etiam tua sponte propensus es? Sapiens autem semper beatus est et est aliquando in dolore; Immo videri fortasse. Paulum, cum regem Persem captum adduceret, eodem flumine invectio? Et ille ridens: Video, inquit, quid agas;&lt;/p&gt;&#039;
- value: &#039;&lt;p&gt;Quae cum dixisset, finem ille. Quamquam non negatis nos intellegere quid sit voluptas, sed quid ille dicat. Progredientibus autem aetatibus sensim tardeve potius quasi nosmet ipsos cognoscimus. Gloriosa ostentatio in constituendo summo bono. Qui-vere falsone, quaerere mittimus-dicitur oculis se privasse; Duarum enim vitarum nobis erunt instituta capienda. Comprehensum, quod cognitum non habet? Qui enim existimabit posse se miserum esse beatus non erit. Causa autem fuit huc veniendi ut quosdam hinc libros promerem. Nunc omni virtuti vitium contrario nomine opponitur.&lt;/p&gt;&#039;
url:
- &#039;http://example.com&#039;
- &#039;http://test.com&#039;
webform_email_confirm:
- &#039;example@example.com&#039;
- &#039;test@test.com&#039;
- &#039;random@random.com&#039;
webform_email_multiple:
- &#039;example@example.com, test@test.com, random@random.com&#039;
webform_time:
- &#039;09:00&#039;
- &#039;17:00&#039;
') (Line: 936)
Drupal\webform\WebformTranslationConfigManager->alterTextareaElement(Array, 'yaml') (Line: 891)
Drupal\webform\WebformTranslationConfigManager->alterSchemaElementsRecursive(Array, Array) (Line: 880)
Drupal\webform\WebformTranslationConfigManager->alterSchemaElementsRecursive(Array, Array) (Line: 859)
Drupal\webform\WebformTranslationConfigManager->alterTypedConfigElements(Array, 'webform.settings') (Line: 143)
Drupal\webform\WebformTranslationConfigManager->alterConfigSettingsForm('webform.settings', Array) (Line: 111)
Drupal\webform\WebformTranslationConfigManager->alterForm(Array, Object) (Line: 67)
webform_form_config_translation_add_form_alter(Array, Object, 'config_translation_add_form') (Line: 460)
Drupal\Core\Extension\ModuleHandler->alter(Array, Array, Object, 'config_translation_add_form') (Line: 893)
Drupal\Core\Form\FormBuilder->prepareForm('config_translation_add_form', Array, Object) (Line: 305)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->{closure:Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber::wrapControllerExecutionInRenderContext():121}() (Line: 633)
Drupal\Core\Render\Renderer::{closure:Drupal\Core\Render\Renderer::executeInRenderContext():633}()
Fiber->start() (Line: 634)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->{closure:Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber::onController():96}() (Line: 183)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 36)
Drupal\Core\Test\StackMiddleware\TestWaitTerminateMiddleware->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 118)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 92)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 54)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 745)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
require('/Users/phen/Sites/drupal/index.php') (Line: 110)
That's quite a lot, and it's hard to pin down. Some invalid YAML is here...somewhere.
phenaproxima → created an issue.
I'm happy to take that on in a separate issue. :)
🤔 A thing that is interesting to me here is that, if the parent class has string $name, and a subclass has $name, PHP's contravariance rules should actually allow that, because subclasses can "widen" parameter types. This has been true since PHP 7.4.
I suspect that this is a really unfortunate collision of the fact that this problem is happening in a trait, not a subclass. That's probably why core felt they could make this change in a patch release (they'd be saved by contravariance), but Webform implementing this method in a trait probably means that contravariance protection was skipped -- a flaw in the PHP engine, perhaps? -- and breaks things.
So that's my analysis. It's a theory, but not an unsound one. This should still be committed; I don't really see another way past it.
This may be breaking people currently using Webform on Drupal CMS. No bueno! Tagging accordingly.
This breaks compatibility with core, apparently...! +1000 RTBC.
Assigning to Lanny per his request.
OK - we've got a test and it's rightfully failing. Handing this off to someone who might have a better of how to extract the Storybook stuff from the mainline CSS files.
Responding to #440: That's a limited list; it may be needed to access JSON schemata stored in .json files. I would suggest handling the file extension limitation to a follow-up, rather than committing this with a too-limited list that needs to be hastily altered for this to be usable in 11.3.
phenaproxima → made their first commit to this issue’s fork.
I made a few changes -- not really to the implementation, but more to how the code is organized. I'd love another look at this, if it passes muster I'll gladly commit it.
phenaproxima → made their first commit to this issue’s fork.
I love how a single line (doesn't even need test coverage!) can be such a small, but significant, nicety to users. Merge train started for 8.x-2.x, thanks!
This seems like a reasonable improvement; there's either a crop for the given URI, or there isn't. Merged into 8.x-2.x, thanks!
One small question here. If it's a bug that you can add a crop effect without any crop types, maybe the crop_type field should be required?
Otherwise this looks good to me.
phenaproxima → made their first commit to this issue’s fork.
phenaproxima → made their first commit to this issue’s fork.
I have a few relatively small suggestions. I think the test could be significantly cleaned up, as well, but that can be done later.
Wrote a passing test. I think, if this passes muster with manual testing, I'd be glad to commit and release it.
I have the best words. Thanks for knocking this one out Wim!
Nice idea, @alexpott. That's a lot cleaner!
For transparency's sake - @mglaman asked me to look at this and validate his comment in #6. I have to admit I have the same question; at first blush, it is not clear to me how invalidating the cache tags of (let's say) media types will also trigger a cache tag invalidation for blocks and SDCs, although I could see a way (potentially) to rig that up by refactoring setCachedDefinitions() in both plugin managers.
But where does that leave code components? Those are just entities, so they're not centrally cached anywhere. How can we possibly trigger an update for them if a new media type is created, apart from just a regular hook_media_type_insert() hook implementation?
If that's the vision, then it would be good to get explicit confirmation about that from Wim.
I think I now disagree with myself in #5, where I said that this needs to be a stable blocker.
This prop source can safely assume it's using the canonical, absolute URL. Why not just keep that assumption, but allow it to hold more options later? I don't see why that needs to result in a nasty update path. host-entity-url with no other options can be an alias, effectively, for "canonical absolute URL". If we add additional options later (and support different relationships), as I sketched out in #5, so be it. We wouldn't need to update every single component tree to make that explicit. (Would we?)
Leaving the tag on to see if Wim agrees, or if I'm missing something, but I just don't see a reason why these feature additions need to block a stable release.
Fixed with a test (confirmed that it fails without the appropriate fix).
Gave this another look. The comments make fairly clear what's going on here, and the MaybeUrl value object makes sense as something we can adapt in the future if we run into a similar need for a "this could a useful value, or it could NULL because of an access problem" case. I appreciate the added clarity (in the class doc comment) about how Evaluator will handle a required value that fails to produce a value. These are really complex problems, and I hope that core will be able to improve the status quo over time, but for now...
...ship it.
Did it a different way, and added tests (called it!). Merged into 2.x and cherry-picked to both 2.0.x and 1.2.x.
phenaproxima → made their first commit to this issue’s fork.
Looks like there are merge conflicts here.
This is not, to be clear, a Drupal CMS stable blocker, and should not be a Canvas stable blocker (or close to it).
Okay, looks like I can't merge this without it being approved by the correct code owners. Boo!
This is great, and useful, and clear as a bell. I'm shipping it with my newfound commit powers! Mwahahahahahaha!!!
Merged into 2.x and cherry-picked to 2.0.x; backported the SiteExporter change to 1.2.x in https://git.drupalcode.org/project/drupal_cms/-/commit/e556faf338197cb86...
That is a lot of excellent and valuable feedback, thanks Wim!
The one point I'd push back on is the hesitancy about "layout". Although I could be convinced otherwise, I feel fairly strongly that "layout" is the clearest term to illustrate the points to people who are new to the internals of Canvas.
I have almost nothing left to complain about; just moving a couple of comments around, and adding a little more high-level explanation to one thing. But otherwise, I say SHIP THIS CHUNGUS!
This is a great idea but I think it needs to be cleaned up in some ways.
I'm happy with this. I ended up removing the unit test because, after the simplifications we made, it's not really testing anything worthwhile. The kernel test remains.
So unfortunately this is completely blocked by ✨ Dispatch ImportEntityEvent during content import Active , because crops reference an entity by its serial ID, and not using an entity reference.
Hmmm...maybe not. If ::normalizePropShape() isn't idempotent, that's...odd.
To be clear, the import fails in the presence of Drupal Canvas, because it adds quite a few validation constraints on preexisting core structures. But, I don't see any reason why we shouldn't clean up the filenames on export to get around this.
Well I'm pretty happy with this. I have one outstanding question about a test case to ensure that the stream wrappers cannot be tricked into accessing absolute paths, like /etc/passwd, that they shouldn't. But if I'm mistaken about that, then I trust the judgment of the more security-minded folks in this issue.
I like and support this change, let's land it!
This seems incredibly straightforward. My complaints are of the easily-swatted-away variety: missing return type hints, unnecessary verbosity in the test coverage, minor capitalization issues, etc. One legitimate question about final.
Kicking back for these small things, but otherwise I'd be entirely comfortable RTBCing this. Seems like a long-needed improvement to me!
I kinda wonder if we should take this further and convert other common characters (like parentheses and ampersands) to hyphens as well.
I think this is exacerbated by Canvas, which adds validation to things that previously had none, including core things.
But either way, I think it makes perfect sense for spaces in file names to be converted to hyphens.
I think I have one major concern here -- if someone does the bare-metal Composer setup that you did, but they also have DDEV installed...ddev launch won't work, even if you have DDEV installed.
I was thinking that maybe the DDEV line should be changed to refer to https://docs.ddev.com/en/stable/users/quickstart/#drupal-drupal-cms instead, but even that is wrong -- if you've done composer create-project, you shouldn't be doing those instructions either.
One option would be to ship a .ddev/config.yaml in the project template, and add a composer quick-start:ddev command as well as quick-start, which does the appropriate stuff for DDEV. But even then...I dunno. This can easily spiral out of control.
This kind of complexity is why we had originally removed any hint of what to do from Composer's output initially, and steered everyone towards DDEV (or the launcher). But as you point out...the download page is inconsistent. It gives you a Composer command that won't really work with DDEV.
Annoyingly, due to the way the repo is laid out and how Drupal CMS's release process works, testing this is cumbersome. But you could do it. Here's how:
- Clone the MR branch somewhere.
- Run this command, outside of the repo root (note that the quoting and the JSON string are important so need to be exactly this):
composer create-project drupal/cms:2.x-dev -s dev --repository='{"type":"path","url":"/PATH/TO/REPO/CHECKOUT/project_template"}'
That should spin up the modified project, and give you the updated message. composer quick-start should work within it.
OK, did this. It pretty much looks like what's in #9 and should work equally well for both the cPanel and normal project template variants. Kicking this fine idea over to Pam for sign-off.
Looks like Drupal's built-in quick-start will work more or less as expected. What do you think of this as the proposed message?
Yeah, it doesn't exist yet. :) But this issue is perhaps the one where we add it!
In the meantime, try https://github.com/drupal/cms-launcher -- another phenaproxima mad science project gone right. (You get bonus points if you somehow manage to break that.)
I might be overthinking this. I know core has a serve or run-server command, it might be smart enough for this.
Self-assigning to do some experimenting. Whatever we do here should be backported to 1.2.x, because why the hell not?
What ChatGPT suggested is pretty much what I think might work here. We could add something like a custom composer serve command, for folks like you who might be using a bare-metal PHP setup. But the thing is, I don't want DDEV people to use that; the command for them is just ddev launch.
We could, of course, just put that in the project message. Something like:
If you're using DDEV, run `ddev launch` to open Drupal CMS. Otherwise, run `composer serve` to get going.
...and the official docs could be updated to mention that.
What do you think?
Hmmm, this could be a tricky one. Could be a permissions thing, or something going subtly wrong during the installation process. That's happened to us before. Assigning to myself to take a peek.
I'm actually gonna mark this one as a stable blocker. We should be able to move to PHP 8.4, and test it, without problems.
Also retitling the issue to be slightly less gross. :)
ANGIE!!!!!!!!!!!!!! ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️
You're right, this oughta be improved. The problem is that the project-message plugin which displays that message is real dumb and has no way to know if you're running under DDEV, or what.
I wonder if maybe we can take advantage of core's quick-start command or something like that.
To recap how we got here...
- Drupal CMS needed this functionality, 100%. We have listing pages that display cards (teasers) of nodes, and link the titles to the full node: an extremely common pattern that will be necessary for Canvas to be useful to site builders.
- Originally, Drupal CMS used its shim module to provide an adapter plugin to show the entity URL. This was strongly pushed back on, since I was warned that the use of adapters would break the Canvas UI.
- So we switched to a computed field (I think it might have been @effulgentsia's suggestion). That worked fine, but it was felt that the feature really had to be in Canvas, not Drupal CMS's helper module, or we'd have to maintain it forever.
- Then you suggested that a computed field was going to be painful because it alters the data model (which made sense at the time), so invented the HostUrlEntity prop source, which was rushed in so that Drupal CMS could take advantage of it by Canvas' RC.
Now, if we actually do need to traverse a data tree to get to URLs at any point in the tree, a new computed base field does make sense. We'd have to implement it on nodes to start with, then expand that to other entity types as necessary.
I can see us having use for something like this: $node->url->canonical->setAbsolute()->setOption('query', ['foo' => 'bar'])->toString()
I could also see (hopefully) core adopting that at some point in the future.
So...count me in, I guess.
Well, wait a sec. Wouldn't that just be a normal entity reference field-powered link? Surely that would be done with a dynamic prop source (perhaps with an adapter on top of it), since those can target any property of any field at any level of a reference chain.
Makes sense to me! Thanks for the detailed comments.
Still needs tests but the initial approach could use a look.
The current test failures are making no sense, and I am flailing in my attempts to fix them. I don't think I'm the right person to handle this one specific test failure, so un-assigning myself for now.
Did the straight rename, but this will need tests to prove that PropSource::parse() will spit out an EntityFieldPropSource if you use the dynamic prefix.
The nature of the change here is straightforward but the code could maybe use a touch of a documentation and a small refactor for clarity.
With Wim's encouragement, I also added PropSource::getPrefix(), which does an inverse match to map a PropSourceBase object to its prefix. This allowed me to remove most implementations of PropSourceBase::getSourceType() in favor of a generic implementation, and it also let me harden some code in ComponentInputs that was hard-coding knowledge of how prop source type strings are put together.
That's a big enough change that this could use another review.