[Webkit-unassigned] [Bug 74749] Runtime changes to requiredExtensions don't update the enclosing <switch>

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=74749





--- Comment #6 from Nikolas Zimmermann <zimmermann at kde.org>  2011-12-21 01:07:21 PST ---
(From update of attachment 120073)
View in context: https://bugs.webkit.org/attachment.cgi?id=120073&action=review

Great catches, I still have some questions:

> LayoutTests/svg/custom/requiredExtensions-runtime-changes.svg:36
> +        var removeElements = document.getElementsByClassName("removeRequiredExtensions");
> +        var addElements = document.getElementsByClassName("addRequiredExtensions");

Clever idea, I keep that in mind for further tests of mine :-) Makes the test itself much more readable/hackable.

> Source/WebCore/ChangeLog:10
> +        SVGStringList, because SVGStringList::reset() adds a single empty string to the list.

Good catch - but I can't recall why reset() behaves this way. Can we just change the behavior of reset?

> Source/WebCore/ChangeLog:13
> +        Invalid elements inside <switch> are prevented from creating renderers,
> +        but are still attached; SVGTests assumes that invalid elements will always be detached.

Why is this? I assumed that attached() returns false, if there's no renderer.
I just checked the code, and indeed this assumption is just wrong.

We're differentiating between two states in dom/Node.cpp (see attach()):
* attach() called no renderer created: attached() returns true and renderer() zero.
* attach() called renderer got created: attached() returns true and renderer() non-zero.

Does anyone know where exactly the former is needed?

When reading common code in eg. ContainerNode, it seems attached() is used to check whether the elements was attached to the rendering tree (which is what I assumed):
...
        // Add child to the rendering tree.
        if (attached() && !child->attached() && child->parentNode() == this) {
            if (shouldLazyAttach)
                child->lazyAttach();
            else
...

    // Remove from rendering tree
    if (oldChild->attached())
        oldChild->detach();
...

If theres a need to differentiate between those states, I'm fine with your approach, otherwise we should rethink it.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.