Follow-up to #2451411: Add libraries-override to themes' *.info.yml
Problem/Motivation
I have added this libraries-override to my theme:
libraries-override:
core/html5shiv: yaml/html5shiv.printshiv
core/normalize: false
and now I searched the source code for normalize and ,core\/html5shiv,core\/normalize, is found. Since I have replaced the one lib is should be updated to yaml/html5shiv.printshiv and the core/normalize need to be removed.
Proposed resolution
Ensure removed libraries are removed from drupalSettings.
Remaining tasks
Add test coverage
User interface changes
None.
API changes
None.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #25 | reroll_diff_4-25.txt | 1.68 KB | sahil.goyal |
| #25 | 2642046-25.patch | 2.2 KB | sahil.goyal |
| #24 | 2642046-nr-bot.txt | 155 bytes | needs-review-queue-bot |
| #4 | libraries_override_does-2642046-4.patch | 2.24 KB | joelpittet |
Comments
Comment #2
hass commentedComment #4
joelpittetThe core committers (@xjm, @alexpott, @catch, @effulgentsia, @Cottser) and theme maintainers (@lauriii, @joelpittet) discussed this issue on April 27th regarding major issues in the theme system.
We were trying to see if there was really major issue here and I think we need more information to that effect. Could you explain why this is a major issue @hass? I can reproduce your issue so that is great but I don't see how this is major and not just normal.
I'm going to send back to normal priority until someone can explain the majorness but also I have a patch to hopefully help resolve the issue in question.
Comment #5
lauriiiI'd like to get feedback from the Javascript subsystem maintainers what is this feature used for? This would help us to figure out what is the expected behavior.
Comment #6
joelpittetNeeds automated regression testing as well. Though I'd like to know what the point of outputing the libraries into the HTML response is being used for as well.
Comment #7
lauriiiSwitching back for the review
Comment #9
nod_I would need some more time to look this up but it's present in drupalSettings for the ajax framework to do it's job.
The list of libs is used to figure out what else the ajax framework needs to load. Having them on the page means that the ajax framework doesn't need to load those 2 again. The first one was overridden and properly overridden. The second one hasn't been loaded but tricks the ajax framework into not loading it in a future request.
Unless the wrong files are loaded this is not a bug.
Comment #10
joelpittet@nod_ If it's overridden to be removed, it shouldn't show in the markup.
Comment #11
nod_I'd say javascript is not markup and wiggle out of this predicament. It's not in my queue anymore so if people feel like doing some PHP, I won't stop them :)
Comment #12
joelpittetThanks for your feedback @nod_, still valuable in it's own right but I think we need to resolve this and that's also why I moved it out of your queue;) Thanks to the fuzziness that are lines between our components:P
Comment #19
larowlanTriaged as part of bug smash initiative
Comment #24
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #25
sahil.goyal commentedRerolled the patch as latest patch #4 was no longer compatible with the 9.5.x so made it compatible with it.
Comment #26
sahil.goyal commentedComment #27
smustgrave commentedDid not test but was previously tagged for test which still need to happen.