Problem/Motivation

As part of making this module ready for Drupal 9, we should add a missing core_version_requirement key to the module info file.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#10 3108883-10-5-interdiff.txt1.19 KBmbovan
#10 3108883-10.patch11.76 KBmbovan
#6 3108883-5-2-interdiff.txt617 bytesmbovan
#5 3108883-5.patch12.95 KBmbovan
#2 3108883-2.patch12.11 KBmbovan

Comments

mbovan created an issue. See original summary.

mbovan’s picture

mbovan commented
Status: Active » Needs review
StatusFileSize
new 3108883-2.patch12.11 KB

This patch adds a necessary core_version_requirement key.

nathandentzau’s picture

nathandentzau
Location New York City
commented

@mbovan your patch contains changes that are not related to the core_version_requirement key change. Could you provide some context as your reason for the change and if that should be moved to a different issue?

mbovan’s picture

mbovan commented 
  1. +++ b/tests/src/Functional/SchemaMetatagTagsTestBase.php
@@ -45,6 +46,11 @@ abstract class SchemaMetatagTagsTestBase extends BrowserTestBase {
+  /**
+   * {@inheritDoc}
+   */
+  protected $defaultTheme = 'stark';

    This is needed to make tests passing. 

  2. +++ b/tests/src/Functional/SchemaMetatagTagsTestBase.php
@@ -107,7 +113,7 @@ abstract class SchemaMetatagTagsTestBase extends BrowserTestBase {
-      list($config_path, $rendered_path, $save_message) = $item;
+      [$config_path, $rendered_path, $save_message] = $item;

    This was an IDE change. Might not be related. 

  3. +++ b/tests/src/Functional/SchemaMetatagTagsTestBase.php
@@ -202,7 +208,7 @@ abstract class SchemaMetatagTagsTestBase extends BrowserTestBase {
-      $json = json_decode($elements[0]->getHtml(), TRUE);
+      $json = Json::decode($elements[0]->getHtml());

    It seems like json_decode() could be deperecated in the future.

mbovan’s picture

mbovan commented
StatusFileSize
new 3108883-5.patch12.95 KB

Reverted #4.2.

mbovan’s picture

mbovan commented
StatusFileSize
new 3108883-5-2-interdiff.txt617 bytes

The interdiff.

nathandentzau’s picture

nathandentzau
Location New York City
commented

Hey @mbovan thank you for the context. Sounds good about fixing the test! I just reviewed this locally and works great. RTBC+++

nathandentzau’s picture

nathandentzau
Location New York City
commented
Status: Needs review » Reviewed & tested by the community
berdir’s picture

berdir
Primary language German
Location Switzerland
commented 
+++ b/src/Plugin/metatag/Tag/SchemaSpeakableTrait.php
@@ -55,16 +55,17 @@ trait SchemaSpeakableTrait {
       '#default_value' => !empty($value['xpath']) ? $value['xpath'] : '',
-      '#description' => $this->t('ex: @example',
-        ['@example' => "'/html/head/title'", "'/html/head/meta[@name=\'description\']/@content']'"),
+      '#description' => $this->t('Separate xpaths by comma, as in: @example',
+        ['@example' => '/html/head/title, /html/head/meta[@name=\'description\']/@content']
+      ),
     ];
 
     $form['cssSelector'] = [
       '#type' => 'textfield',
       '#title' => $this->t('cssSelector'),
       '#default_value' => !empty($value['cssSelector']) ? $value['cssSelector'] : '',
-      '#description' => $this->t('ex: @example',
-        ['@example' => "'#title'", "'#summary'"]
+      '#description' => $this->t('Separate selectors by comma, as in @example',
+        ['@example' => '#title, #summary']
       ),
     ];

Would be better to leave this out of this patch to avoid conflicts with the other issue.

mbovan’s picture

mbovan commented
StatusFileSize
new 3108883-10.patch11.76 KB
new 3108883-10-5-interdiff.txt1.19 KB

Definitely, the snippet was accidentally added in #5 from #3068796: Fatal PHP syntax error in SchemaSpeakableTrait.php.

mbovan’s picture

mbovan commented

#10 https://www.drupal.org/pift-ci-job/1555336 failed with an error that should be fixed in #3068796: Fatal PHP syntax error in SchemaSpeakableTrait.php.

karens’s picture

karens commented
Status: Reviewed & tested by the community » Closed (duplicate)

I missed this issue and am fixing this in #3138582: Drupal 9 compatibility. Sorry!