[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 #7 from Tim Horton <timothy_horton at apple.com>  2011-12-21 10:44:13 PST ---
(In reply to comment #6)
> (From update of attachment 120073 [details])
> 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?

I'd love to! I couldn't come up with a good reason (as it stands, this makes the empty-hasn't-been-touched list and the empty-was-touched-but-got-cleared list different, and I can't see a good reason for that, at all).

> > 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.

I have no idea if there's a reason for this. I assumed there was, and just wanted to make our assumptions fit. Do you know who might know?

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