Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Numeric fields are meant to have a "content=value" attribute set on the field-item when a prefix or suffix is added to the number for display. This attribute will currently not be added when the field-item doesn't contain other attributes. It is irrelevant whether there are other attributes or not, so the test should be changed.
Steps to reproduce
- Standard Install of Drupal
- Add a Number (integer) field to the Article content type
- Configure the field to have a prefix
- Create content with a number in the integer field and notice that the markup is missing the "content=value" attribute on the field-item wrapper around the number
Proposed resolution
I think the intention was to make sure the _attribute property is set before adding to it. Instead of if (!empty($item->_attributes) ... replace with if (isset($item->_attributes)....
After the patch, the correct markup is output as shown in the screenshot.
Remaining tasks
Review patch.
User interface changes
None.
API changes
None
Data model changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | interdiff-2537288-8.txt | 1.22 KB | segi |
| #9 | 2537288-attribute-for-numeric-8.patch | 3.14 KB | segi |
| #3 | content_value_attribute-2537288-1-after.jpg | 59.87 KB | HeimdallJHM |
| #3 | content_value_attribute-2537288-1-before.jpg | 58.7 KB | HeimdallJHM |
| #1 | 2537288-correct-markup.png | 8.75 KB | ShaunDychko |
Comments
Comment #1
ShaunDychko CreditAttribution: ShaunDychko as a volunteer commentedComment #2
Anonymous (not verified) CreditAttribution: Anonymous commentedLooks like a reasonable patch to me.
Comment #3
HeimdallJHM CreditAttribution: HeimdallJHM at ASPgems commentedTested. As you can see in the attached images the patch works
Comment #4
HeimdallJHM CreditAttribution: HeimdallJHM at ASPgems commentedComment #5
alexpottThis should be tested.
Comment #6
ShaunDychko CreditAttribution: ShaunDychko as a volunteer commentedComment #7
ShaunDychko CreditAttribution: ShaunDychko as a volunteer commentedAutomated tests added. I included a patch with new tests only, one of which fails as expected, and then included a combined patch with both the new tests and the fix (following https://www.drupal.org/contributor-tasks/write-tests).
Comment #9
segi CreditAttribution: segi at Cheppers commentedI reviewed the patch and it gives the necessary content attribute to DOM. I think the patch is correct, I found a little problem which is maybe only problem for me, but I think one variable name "$the_value" is not too clear what it contains so I changed it to $integer_value.
Comment #10
ShaunDychko CreditAttribution: ShaunDychko as a volunteer commentedScreenshots in #3, review/slight revision of tests in #9. RTBC.
Comment #12
subhojit777Seems to me like a random failure. Invoking the bot again.
Comment #14
ShaunDychko CreditAttribution: ShaunDychko as a volunteer commentedPasses again, so setting back to RTBC.
Comment #15
ShaunDychko CreditAttribution: ShaunDychko as a volunteer commentedComment #16
alexpottCommitted e249d75 and pushed to 8.0.x. Thanks!