1867225 - SVGUseElement::mOriginal usage is suspicious / probably wrong.

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

This is done regardless of whether the clone is for <svg:use> or it is a regular .cloneNode call from script.

It should probably be doing it only if aNodeInfo->GetDocument()->CloningForSVGUse().

https://crisal.io/tmp/svg-use-original.html is an example that shows wrong behavior with the first version of the patch for bug 1806964. I'm fairly sure you can make it misbehave in some way in release too, but I haven't spent enough time trying to do it.

Robert does the proposal above sound good? Am I missing something?

With that and something like https://hg.mozilla.org/try/rev/2ea7097c0966a4819dc30abb17ab28dca9d00210#l2.35 seems we could go back to using ResetWithId, wdyt?

Comment 1

[Robert Longson [:longsonr]]

Sounds worth trying, yes.

Comment 2

[Robert Longson [:longsonr]]

it's all about where you find the thing use points to when it's a clone.

use A-> use B->rect

use B exists as a clone and an original. When its clone renders in order to render use A then the clone needs to find the rect which will be in use B's document if use B has a local href to rect.

For .cloneNode as part of the shadow DOM where do we expect to find the rect? If that's always also in the shadow DOM because it's also cloned through the cloneNode then setting mOriginal only for CloningForSVGUse() is the right thing to do.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1867225 - Use ResetWithID for local SVG use references. r=longsonr

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fa1d01606045
Use ResetWithID for local SVG use references. r=longsonr

Comment 5

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/fa1d01606045