Problem/Motivation

Do not modify xmlns values in svgs or other xml, only reference links

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3462868

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mikelutz created an issue. See original summary.

annmarysruthy’s picture

Assigned: Unassigned » annmarysruthy

annmarysruthy’s picture

Assigned: annmarysruthy » Unassigned

annmarysruthy changed the visibility of the branch 3462868-replacehttp-1 to hidden.

annmarysruthy’s picture

Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Novice

Seems to be some missed one

views.theme.inc
breakpoint.module
breakpoint.overview.html.twig

maybe a few more.

sheetal-wish made their first commit to this issue’s fork.

sheetal-wish’s picture

Status: Needs work » Needs review
ankitv18’s picture

Status: Needs review » Needs work

I found this reference http://www.w3.org in total 80 files and MR consists of changes in 52 files
Seems more files need to be taken care of.

nexusnovaz’s picture

Assigned: Unassigned » nexusnovaz

Ill take a quick look and see if i can get the rest

nexusnovaz’s picture

Assigned: nexusnovaz » Unassigned
Status: Needs work » Needs review

Found two more files which matched the problem. There is one last file which is contained in ckeditor5-dll.js. As this wasn't a normal file, i've left that one to be discussed. There are another 100 matches however, those are xmlns references.

ankitv18’s picture

StatusFileSize
new243.96 KB

Changes looks good but not sure whether we need to consider .twig .php files having http://www.w3.org (Within svg xmlns)
Attaching screenshot for your reference.
Removing http reference

ankitv18’s picture

Also synced the forked branch with 11.x

smustgrave’s picture

Think svgs should be a novice follow up

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs followup
smustgrave’s picture

Also @ankitv18 please leave this for the newer users who were already working on it thanks

ankitv18’s picture

@smustgrave, I just reviewed and synced the forked branch

yujiman85’s picture

Assigned: Unassigned » yujiman85

Taking this on to make the other changes.

yujiman85’s picture

Assigned: yujiman85 » Unassigned
Status: Needs work » Needs review

I wasn't sure if the .js files needed to be touched since those seemed compiled and minified. Please correct me if I'm wrong. This needs a review.

smustgrave’s picture

Status: Needs review » Needs work

Welcome @Yujiman85 and nice work!

Appears to need a rebase now though. Will keep an eye out for your changes.

quietone’s picture

This is still a very large change. I am guessing a script is used to do this? Can it be shared?

yujiman85’s picture

Status: Needs work » Needs review

@smustgrave Alright, I believe I did this right. I rebased the branch and it is up to date now with the commits from this branch. Needs a review.

ankitv18’s picture

Status: Needs review » Needs work

Pipelines aren’t passing ~~ hence moving back into NW

rodrigoaguilera’s picture

Novice issue reserved for the Mentored Contribution during DrupalCon Barcelona 2024. After the 27th of September 2024, this issue returns to being open to all. Thanks

I think this needs a manual rebase since the "/rebase" gitlab command fails. The current pipeline fail doesn't make sense to me

ultimike’s picture

I will be working on this issue during the DrupalCon Barcelona mentored contribution with @pierregermain

After September 28, 2024, feel free to pick this issue up and continue.

-mike

pierregermain’s picture

Rebased pierregermain made their first commit to this issue’s fork.

pierregermain’s picture

Status: Needs work » Needs review

We reverted the last commit that was introducing changes to svg files that made the pipeline fail. After reverting the commit, the pipeline is passing again. Needs Review.

lostcarpark’s picture

Status: Needs review » Reviewed & tested by the community

I have reviewed this issue and verified all the changes are to reference links, not XML or SVGs. All tests are passing.

Moving to RTBC.

  • quietone committed 3359812d on 10.4.x
    Issue #3462868 by annmarysruthy, yujiman85, nexusnovaz, pierregermain,...

  • quietone committed 9ba6f744 on 11.x
    Issue #3462868 by annmarysruthy, yujiman85, nexusnovaz, pierregermain,...
quietone’s picture

Version: 11.x-dev » 10.4.x-dev
Status: Reviewed & tested by the community » Fixed

I applied the diff and there are 4 occurrences, all of which are not to be modified. git grep http://www.w3.org | grep -v .svg

  1. core/modules/media/tests/fixtures/example_1.jpeg:
  2. core/modules/media/tests/fixtures/example_2.jpeg:
  3. core/modules/navigation/tests/assets/image_test_files/test-logo.png:
  4. core/phpunit.xml.dist:

Committed and pushed to 11.x and 10.4.x. Thanks!

Status: Fixed » Closed (fixed)

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

quietone’s picture

Issue tags: -Needs followup

The comment #3399840-11: [meta] Replace http urls with https urls of the respective sites in core explains why the SVGs should not be changed, so I am removing the tag for a followup.