Support RDFa output in number field formatter

We need to extend the tests to cover the case when there is a prefix or a suffix and also thousand separators. This will also require to add some logic in the viewElements methods, I think we can achieve this by pushing a @content attribute to the field template similar to #2188889: Support RDFa output in link field formatter.

+++ b/core/modules/rdf/src/Tests/Field/NumberFieldRdfaTest.php
@@ -0,0 +1,116 @@
+  protected function createTestField($field_settings = array()) {

no longer needed I believe, since you fixed the parent class method.

+++ b/core/modules/rdf/src/Tests/Field/NumberFieldRdfaTest.php
@@ -0,0 +1,116 @@
+  protected function createMapping($field_settings = array()) {

We're doing more than creating a mapping here. I would name this method createTestEntity and take the createTestField() out of it.

+++ b/core/modules/rdf/src/Tests/Field/FieldRdfaTestBase.php
@@ -105,7 +105,7 @@ protected function assertFormatterRdfa($formatter, $property, $expected_rdf_valu
+  protected function createTestField($field_settings = array()) {

$field_settings should be called $field_instance_settings to be more accurate.

+++ b/core/modules/rdf/src/Tests/Field/NumberFieldRdfaTest.php
@@ -0,0 +1,96 @@
+        'url_plain' => FALSE,

url_plain is irrelevant here.

+++ b/core/modules/rdf/src/Tests/Field/NumberFieldRdfaTest.php
@@ -0,0 +1,157 @@
+  /**
+   * Create the RDF mapping for the field.
+   */

Creates

Looks like #32 is only using the xpathContent() on the testFloatFormatter() test, is that intentional? Seems to me we should be using on pretty much all the tests to see whether the @content is there or not. It should be there when we expect the value to differ from the one in the HTML element, and it should check that it's not there when the values are the same, no?

Patch looks great otherwise, I really like how this issue is evolving!

1. +++ b/core/modules/rdf/src/Tests/Field/NumberFieldRdfaTest.php
   @@ -31,6 +31,10 @@ public function testIntegerFormatter() {
   +    // Test that the content attribute is not created.
   +    $result = $this->xpathContent($this->getRawContent(), '//div[contains(@class, "field-item") and @content=:testValue]', array(':testValue' => $this->testValue));
   +    $this->assertFalse($result);
   

   Looks good, but I'd rather test for the presence of the content attribute period (regardless of the value is has).

2. +++ b/core/modules/rdf/src/Tests/Field/NumberFieldRdfaTest.php
   @@ -53,6 +57,10 @@ public function testIntegerFormatterWithSettings() {
   +    // Test that the content attribute is not created.
   +    $result = $this->xpathContent($this->getRawContent(), '//div[contains(@class, "field-item") and @content=:testValue]', array(':testValue' => $this->testValue));
   +    $this->assertTrue($result);
   

   In the positive case, I agree we should test the value of the content attribute. The comment needs to be fixed though, and the same applies to the other instances of the assertTrue() of the content attribute (comment is inaccurate).

1. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/NumericFormatterBase.php
   @@ -78,6 +78,11 @@ public function viewElements(FieldItemListInterface $items) {
   +      if ($item->value != $output) {
   +        if (!empty($item->_attributes)) {
   +          $item->_attributes += array('content' => $item->value);
   +        }
   +      }
   

   Could add a comment to give some context: Output the raw value in a content attribute if the text of the HTML element differs from the raw value (for example when a prefix is used)

2. +++ b/core/modules/rdf/src/Tests/Field/FieldRdfaTestBase.php
   @@ -131,4 +133,52 @@ protected function getAbsoluteUri($entity) {
   +   * Parse a content and return the html element.
   

   "Parses"

3. +++ b/core/modules/rdf/src/Tests/Field/NumberFieldRdfaTest.php
   @@ -0,0 +1,185 @@
   + * Tests RDFa output by number field formatters.
   ...
   +  /**
   +   * The 'value' property value for testing.
   +   *
   +   * @var string
   +   */
   +  protected $testValue = 'test_text_value';
   

   the testvalue property is already set by the base test class, and we are setting it inside each test method, so I don't think it's needed here at all.

4. +++ b/core/modules/rdf/src/Tests/Field/NumberFieldRdfaTest.php
   @@ -0,0 +1,185 @@
   +    // Test that the content attribute is not created.
   +    $result = $this->xpathContent($this->getRawContent(), '//div[contains(@class, "field-item") and @content]', array(':testValue' => $this->testValue));
   +    $this->assertFalse($result);
   

   Looks good now, though we no longer use the :testValue in the XPath expression anymore, so it can be removed from the arguments. (same for all other negative tests).

5. +++ b/core/modules/rdf/src/Tests/Field/NumberFieldRdfaTest.php
   @@ -0,0 +1,185 @@
   +  /**
   +   * Tests the float formatter with a scale.
   +   */
   +  public function testFloatFormatterWithScale() {
   +    $this->fieldType = 'float';
   +    $formatter = array(
   +      'type' => $this->fieldType,
   +      'settings' => array(
   +        'scale' => 5,
   +      ),
   +    );
   +    $this->testValue = 3.33;
   +    $this->createTestField();
   +    $this->createTestEntity();
   +    $this->assertFormatterRdfa($formatter, 'http://schema.org/baseSalary', array('value' => $this->testValue));
   +
   +    // Test that the content attribute is not created.
   +    $result = $this->xpathContent($this->getRawContent(), '//div[contains(@class, "field-item") and @content]', array(':testValue' => $this->testValue));
   +    $this->assertFalse($result);
   +  }
   

   I'm was surprised this passed and that we didn't have a content attribute with the scale setting, but this is because the scale is not exercised. The test value should include more than 5 digits after the ".". In which case there should be a content attribute.

+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/NumericFormatterBase.php
@@ -78,6 +78,13 @@ public function viewElements(FieldItemListInterface $items) {
+      // Output the raw value in a content attribute if the text of the HTML
+      // element differs from the raw value (for example when a prefix is used).
+      if ($item->value != $output) {
+        if (!empty($item->_attributes)) {
+          $item->_attributes += array('content' => $item->value);
+        }
+      }

These 2 if conditions could go on the same line, with the empty() being first. Otherwise the rest looks good.

Thanks Jamie, great work as usual! Probably the most sophisticated field to test!