Skip to content

fix(prefixIds): does not handle SMIL timing offsets#2208

Open
yangxu52 wants to merge 1 commit intosvg:mainfrom
yangxu52:fix/prefixIds-not-handle-SMIL-offsets
Open

fix(prefixIds): does not handle SMIL timing offsets#2208
yangxu52 wants to merge 1 commit intosvg:mainfrom
yangxu52:fix/prefixIds-not-handle-SMIL-offsets

Conversation

@yangxu52
Copy link
Copy Markdown

This PR improves handling of begin and end attributes in the prefixIds plugin.

Problem

The current implementation only prefixes values ending with .end or .start, but valid SMIL syntax also includes offsets such as:

  • a.end-0.5s
  • b.begin+0.1s

These cases are currently ignored, leading to broken animation references after ID prefixing.

Solution

Replace the suffix check with a regex that correctly parses:
<id>.(begin|end)(+/-offset)?
This ensures all valid SMIL timing references are properly prefixed.

Impact

  • Fixes broken animation chaining
  • Maintains backward compatibility
  • No effect on non-SMIL attributes

Example

Before:

begin="a.end-0.5s"

After

begin="prefix-a.end-0.5s"

Test

  1. without offsets, like x.end, OK
  2. with offsets, like b.end-0.5s, OK
  3. mix and multiple, like x.begin;x.end-0.5s, OK

Test code e.g:

it('should prefix SMIL begin/end with offsets', () => {
  const svg = `
    <svg>
      <animate id="a" begin="b.end-0.5s" />
      <animate id="b" begin="a.begin+0.1s" />
    </svg>
  `

  const result = optimize(svg, {
    plugins: [
      'cleanupIds',
      {
        name: 'prefixIds',
        params: { prefix: 'p' },
      },
    ],
  })
  expect(result.data).toContain('p-b.end-0.5s')
  expect(result.data).toContain('p-a.begin+0.1s')
})
image

Notice
This PR intentionally keeps a minimal regex-based approach consistent with existing implementation, while extending coverage to valid SMIL offset syntax instead of all.

Linked Issue
Fixes: #2207 #2073

- Using the `replace` callback function to handle the offset and else, replacing the original `endsWith` matching
- adjust the RegExp of split to adapt multiple value
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

prefixIds does not correctly handle SMIL begin/end with offsets (e.g. ".end-0.5s")

1 participant