280214 - [FIX]views don't get updated with SVG generated in XUL documents

Status: RESOLVED FIXED

(Core :: SVG, defect, P1)

Description

[Marco Perez [:mpk]]

I suspect the fix for bug #244366 to be the cause for this regression. Builds
with sources checked out using "-D 2005-01-19 19:38:00 PST" were okay wheras the
ones using "-D 2005-01-19 19:40:00 PST" were not.
Other platforms (especially Linux) may be affected as well, but I don't have any
systems to test with right now. I'll attach a simple testcase shortly to
demonstrate the problem.

Comment 1

[Marco Perez [:mpk]]

Attachment: simplified testcase to demonstrate the problem

Changing the svg checkbox rebuilds the content (either xhtml or svg) from
scratch, thereby replacing all the former elements. The second checkbox
controls whether the svg root element should be inserted into the document
right after creation or just after it gets some children first. On my
svg-enabled (libart) firefox builds this results in either one faulty behavior
(no content at all) or another (both xhtml and svg are displayed) when svg is
checked. Deactivation of svg works as it should. svgRootElement.forceRedraw()
is always performed at the end of the initialization, but doesn't have any
visible effects until you perform it manually. I tried to reproduce this bug
with an XHTML host document instead of a XUL document. This ran fine however...

Comment 2

[Boris Zbarsky [:bzbarsky]]

I don't see how forceRedraw() ever worked.... it's just calling the wrong function.

Over to SVG, ccing some SVG folks in case they know why it was done this way.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: This fixes the bug for me

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Same testcase without refresh() call

Though let me see if I understand it correctly.  The patch fixes the testcase
as-is, with the forceRedraw() call after the DOM has been modified.

If I remove that forceRedraw() call, then even in a build with my patch I see
drawing problems.  I assume that's also incorrect, right?  ;)

Comment 5

[Marco Perez [:mpk]]

Well, I put a forceRedraw() call at the end of the script to make sure that the
svg content was up to date. This led me to believe that this bug would belong to
layout rather than svg. The original script didn't have that.
And yes, it would be more than just great if the reflows would take place
without having to force redraws. ;-)
I could put the original code on a publicly accessible web server if requested.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch that fixes the remaining issue

So in brief, the SVG code is pretty evil.  It disables painting for the entire
viewmanager tree when an outer SVGFrame is created, and only reenables it when
it's reflow (from inside reflow!), via Begin/EndUpdateViewBatch.

It also uses Begin/EndUpdateViewBatch to implement the disable/enableRefresh()
DOM methods on SVG nodes.

I've filed bug 280363 on making SVG not misuse the viewmanager api or changing
the viewmanager api to allow the model that SVG seems to be expecting.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Both patches checked in.