Follow-up to #2329599: Update classList.js

Problem/Motivation

The version for one of our JavaScript libraries (classList) is not returned correctly in LibraryDiscoveryParser. It is defined as:

version: 2014-12-13

But when parsing the library file in LibraryDiscoveryParser::parseLibraryInfo(), this is parsed as a UNIX timestamp (1418425200):

 Yaml::decode(file_get_contents($this->root . '/' . $library_file));

Versions should always be strings.

Proposed resolution

Explicitly cast all version numbers as strings by using double quotes for version numbers in the core.libraries.yml file.

Remaining tasks

Review patch

User interface changes

N/A

API changes

N/A

CommentFileSizeAuthor
#3 drupal8-libraries-version_2470928-3.patch6.35 KBandrewsuth

Comments

stefan.r’s picture

stefan.r commented
Component: javascript » asset library system
andrewsuth’s picture

andrewsuth commented
Assigned: Unassigned » andrewsuth

I'm at Drupal Dev Days and will fix this issue.

andrewsuth’s picture

andrewsuth commented
Issue summary: View changes
StatusFileSize
new drupal8-libraries-version_2470928-3.patch6.35 KB

- Changed necessary library versions to strings in order to avoid YAML parsing conversions (in this case, it was found that an ISO date was converted to UNIX timestamp.
- Added a unittest to demonstrate the bug.

dom.’s picture

dom. commented
Status: Active » Needs review
stefan.r’s picture

stefan.r commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +drupaldevdays

Confirmed that all version numbers are double quoted in core.libraries.yml where possible, and that the bug with the invalid classList library version parsing has been fixed.

Regardless of whether we fix the date casting issue elsewhere, it seems like good practice to always double quote version numbers in order to prevent similar issues in the future, so patch looks ready to me!

stefan.r’s picture

stefan.r commented
Assigned: andrewsuth » Unassigned
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 127f0c8 and pushed to 8.0.x. Thanks!

  • alexpott committed 127f0c8 on 8.0.x 
    Issue #2470928 by andrewsuth: Versions in core.libraries.yml are not...
neclimdul’s picture

neclimdul
He/Him
commented

it was found that an ISO date was converted to UNIX timestamp.

Not really a bug. Plain style (untagged) nodes like this are allowed to be resolved in this manner depending on the application. So pecl or symfony are allowed to define behaviors that allow this sort of resolution.
http://www.yaml.org/spec/1.2/spec.html#id2761292

The spec also documented how plain style ISO dates nodes can be resolved to timestamps.
http://www.yaml.org/spec/1.2/spec.html#id2761573

stefan.r’s picture

stefan.r commented

Just to be clear, the bug was the fact that the classList version was not returned correctly -- not the YAML parsing itself.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.