Closed Bug 380516 Opened 14 years ago Closed 14 years ago
[FIX]SVG image not rendered - regression between 3
.0a4pre and 3 .0a5pre
370 bytes, image/svg+xml
1.10 KB, text/html
1.10 KB, text/html
1.12 KB, text/html
1.13 KB, text/html
1.11 KB, text/html
3.05 KB, application/x-zip-compressed
2.46 KB, patch
|Details | Diff | Splinter Review|
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070512 Minefield/3.0a5pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070512 Minefield/3.0a5pre In the attached test case, no image appears with the latest trunk builds. Sometimes the image appears for a moment and then disappears. Reproducible: Always Steps to Reproduce: 1. Open the attached test.html 2. 3. Actual Results: No image appears with the latest trunk builds. Sometimes the image appears for a moment and then disappears. Expected Results: The image, consisting of a rectangle and some text, should be displayed. Works correctly on FF2 and FF3 3.0a4pre. The conditions required are that (i) The SVG is populated from the HTML load handler (OK if loaded from the SVG load handler) (ii) The SVG contains text. (iii) The attribute "font-family" is set on a <g> element after adding the <g> to the root element (OK if don't set "font-family" or set it before adding <g> to root element)
This might possibly be related to another bug of mine, bug 374293, although that bug didn't regress at the same time. It is to do with changing font attributes causing the image to disappear though.
I can see the image after resizing the window. And also if I have the pref browser.display.use_document_fonts set to 0. I found this regression range: http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1178037420&maxdate=1178040899
Status: UNCONFIRMED → NEW
Ever confirmed: true
Duncan: would you be able to take the time to create a version of the test that works correctly (exactly the same rendering as you'd expect from the buggy testcase), without any of the complicating factors that make it fail? It'll make it easier to keep this from regressing in the future.
This version works OK because font-family is set before adding the element to the document.
This version works OK because there is no text in the SVG
This version works OK because the SVG is populated from the SVG load handler rather than from the HTML load handler.
This version appears to work correctly simply by virtue of having an alert message in it. Maybe there is a timing dependency ? Something I haven't been able to explain yet is that not all of my SVG images are affected by this bug. I have some SVG images that do the same thing, setting setting font-family on a <g> etc and they carried on working with 3.0a5pre whilst others broke.
Re-attaching version that works libe
Attachment #264646 - Attachment is obsolete: true
Finger trouble - original had both workarounds 1 and 3
Attachment #264645 - Attachment is obsolete: true
Finger trouble - original had both workarounds 1 and 2
Attachment #264644 - Attachment is obsolete: true
(In reply to comment #5) > Duncan: would you be able to take the time to create a version of the test that > works correctly (exactly the same rendering as you'd expect from the buggy > testcase), without any of the complicating factors that make it fail? It'll > make it easier to keep this from regressing in the future. > Jeff, not sure exactly what you were after but have attached four versions that all work correctly 1 - Set font-family before adding to the document 2 - No text 3 - Populate from SVG load handler instead of html load handler 4 - Adding an alert If you're looking for something to put into a test suite to provide "expected results", avoid 2 as it gives a different result (having no text). Let me know if you need something more or something different.
It's possible this bug might be related to one or both of bug 374293 and bug 375342 which both involve SVG images disappearing with timing dependencies (although those two regressed at an earlier point than this). Not that I'm lobbying to get three of my bugs addressed for the price of one of course.
Having re-read bug 375342, the symptoms are very very similar. I think it would be worthwhile someone taking a look at both of these two together (had no interest in bug 375342 as yet). I've been working around bug 375342 for ages by adding a small delay after setting font-family and that might explain why some of my SVG images were unaffected by this bug.
(In reply to comment #13) > Jeff, not sure exactly what you were after but have attached four versions > that all work correctly Sorry, I could have been clearer. The final goal of the tests is to make them reftests, as described in this (rather long) document: http://developer.mozilla.org/en/docs/Creating_reftest-based_unit_tests I can summarize what's relevant to a test author as follows: - a reftest consists of a test document and a reference document - the test document determines how it looks in a "tricky" way - e.g., looking like a single box of a single color by being made up of the colored borders of a bunch of individual boxes, as seen through colored, alpha-transparent boxes - the reference document determines how it looks in a simple, easy-to-understand way - e.g. just being one box with its colors set using a background: rule - the test passes if one of the following is true (as chosen by the author): - the documents have pixel-equivalent renderings - the documents render differently (useful for, e.g., comparing various text alignments to "justify" to ensure justification is different from centering) Note that success/failure is determined entirely by how the documents render -- alerts can't be used to indicate success or failure because they don't affect rendering. There are a couple other complications related to exactly when renderings are compared, but if you can set everything up from the onload handler you don't need to worry about them. As far as what you've posted goes, except for the alert test it sounds like the tests are fine (I think; I haven't followed this bug super-closely). The only thing I'd suggest is making the pairing between a test and its reference more obvious; currently the attachments aren't particularly organized. Just uploading a zip containing document pairs would probably work, with the test files following this naming convention: 380516-01.html 380516-01-ref.html 380516-02.svg 380516-02-ref.svg 380516-03.xhtml 380516-03-ref.xhtml ... Thanks again for helping!
Jeff, having read up about reftests I think the attached is what you need.
One thing that bothers me slightly is that sometimes these tests might pass because of time dependency. Just now, I've observed the image to render OK on occasion.
The problem is that when we do an incremental reflow of the outer SVG frame (triggered by the style change on the kid), it doesn't set its desired size (which in practice means it collapses to 0x0). I didn't roll the other reftests into this patch, based on the comment about them not being reliable, and on the fact that the reference files are much too complicated. I'd expect to see no script (or HTML) in the reference files; I'd rather expect to not see any HTML in the test files either, unless the bug is completely impossible to demonstrate with a standalone SVG document...
Oh, and I bet my testcase shows the problem with builds from before bug 378975 too -- this looks like a reflow branch regression to me.
OS: Windows XP → All
Priority: -- → P2
Hardware: PC → All
Summary: SVG image not rendered - regression between 3.0a4pre and 3.0a5pre → [FIX]SVG image not rendered - regression between 3.0a4pre and 3.0a5pre
Target Milestone: --- → mozilla1.9alpha6
Comment on attachment 264714 [details] [diff] [review] Proposed fix and simple test sr=dbaron
Attachment #264714 - Flags: superreview?(dbaron) → superreview+
Boris, great if you've fixed this - interesting the fix would suggest an earlier regression than what I observe for this bug. Is there any chance at all the same issue might explain bug 375342 and/or bug 374293 (even though they regressed earlier) ? I think those two and this are all timing dependent which might explain why the exact regression range is hard to pin down.
That is very excellent news. Three for the price of one - that's value for money that is. Out of interest, where do you reckon the time dependency arises from ? Is it that the document will resize anyway at some point (e.g. when the HTML is parsed) and it depends if the SVG rendering completes first (or something like that) ?
The dependency comes from the ordering of the action that posts the incremental reflow event and the initial/resize reflows we do during pageload. If the event fires after those, you see the bug.
Got it. That would explain everything I've seen on all three bugs I think. And why its been such a pig to produce test cases and why the test cases are so complex. And probably why we didn't see this bug regress earlier (something that changed in the latest trunk slightly affecting this ordering and making the bug obvious).
Comment on attachment 264714 [details] [diff] [review] Proposed fix and simple test r=jwatt. Thanks!
Attachment #264714 - Flags: review?(jwatt) → review+
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.