411382 - SVG document with unspecified height gets clipped at 150px when printed

Status: RESOLVED FIXED

(Core :: SVG, defect, P2)

Description

[Martijn Wargers (dead)]

See url, when printing that document, almost nothing shows up as print output, only a few lines.
It just works fine on branch.

Comment 1

[Martijn Wargers (dead)]

I also noticed that print preview looks much smaller on trunk than it is on branch, I filed bug 411383 for that.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: ignore -- posted to wrong bug

Comment 3

[Daniel Holbert [:dholbert]]

Comment on attachment 298607 [details]
ignore -- posted to wrong bug

oops -- sorry, wrong bug

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: printing results

This is the output I get from both Print and Print-Preview on WinVista and on Linux in trunk.  Just the top half of the butterfly is printed, to the base of its head.

Comment 5

[Daniel Holbert [:dholbert]]

Regression range is between these nightlies:

Gecko/2007111704 Minefield/3.0b2pre
Gecko/2007111806 Minefield/3.0b2pre

Bonsai link:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-11-17+03%253A20%253A00&maxdate=2007-11-18+06%253A40%253A00&cvsroot=%252Fcvsroot

Looks like regression from bug 294086.

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (pure SVG)

Here's a somewhat reduced butterfly testcase, with just one of the <path> elements.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (html, no printing required)

Here's the previous SVG testcase embedded as an object in an HTML document.

It looks identical to the printing output -- only the top half of the butterfly is visible.

Some DOM inspection shows that the <object> element is 150px tall.  Maybe we're assuming SVG elements are 150px tall unless a height is specified?

Note that we can hide the bug by adding a specified height (e.g. 300px) to either the SVG source or the <object> element.  So this is a bug with unspecified-width-SVG objects.

Comment 8

[Daniel Holbert [:dholbert]]

In Firefox 2 and in builds up to the regression point, the HTML testcase (attachment 298784 [details]) rendered with...
 - height clipped at 150px
 - width clipped at 300px.

After the regression window, the width clipping is removed, so now just the height clipping remains.

Also -- these issues all occur with this testcase as well:
  http://www.croczilla.com/svg/samples/circles1/circles1.svg
That is, it's clipped to be 150px high when printed, and it's also clipped when included as an <object> in a HTML document.  As I said in my previous comment, this seems to be due to the fact that the testcase's SVG doesn't give a height.

Comment 9

[Jonathan Watt [:jwatt]]

> this is a bug with unspecified-width-SVG objects.

Unspecified width defaults to 100%, so this is rather a problem with percentage width/height during printing. When percentage width can't be resolved it defaults to 300px, and when percentage height can't be resolved it defaults to 150px (as per CSS 2.1 spec.). For some reason it looks like percentages for width/height on root elements can't be resolved (no viewport/containing block).

Comment 10

[Daniel Holbert [:dholbert]]

(In reply to comment #9)
> > this is a bug with unspecified-width-SVG objects.

Sorry -- s/width/height

Comment 11

[Daniel Holbert [:dholbert]]

Attachment: testcase 3 (html, with table wrapper)

This new testcase is the same as the previous one, but with a table wrapping around the SVG <object>.  The table just has a green border, and has no specified width/height.

Here's how we handle this testcase:
  Trunk Pre-411383 (and FF2):  SVG <object> clipped to 150px high, 300px wide
  Trunk Post-411383:           SVG <object> clipped to 150px high, 0px wide
                                 (i.e. only the green border is visible)

So it looks like we're now returning 0px as the default width, not 300px.

Comment 12

[Daniel Holbert [:dholbert]]

(In reply to comment #11)
> So it looks like we're now returning 0px as the default width, not 300px.

(this width issue may be worthy of a separate bug -- just posting it here for its relevance to this bug and in case it's easily fixed as part of this bug)

Comment 13

[fantasai]

Yeah, we don't set the initial containing block correctly for print, see bug 381497.

Comment 14

[Jonathan Watt [:jwatt]]

Comment on attachment 298804 [details]
testcase 3 (html, with table wrapper)

(In reply to comment #11)
> Created an attachment (id=298804) [details]
> testcase 3 (html, with table wrapper)
> 
> This new testcase is the same as the previous one, but with a table wrapping
> around the SVG <object>.  The table just has a green border, and has no
> specified width/height.

Yup, that's bug 382325. See the third attachment (attachment 289669 [details]) on that bug.

Comment 15

[Jonathan Watt [:jwatt]]

Comment on attachment 298784 [details]
testcase 2 (html, no printing required)

> Created an attachment (id=298784) [details]
> testcase (html, no printing required)

This is exactly as CSS 2.1 requires. What happens is that the SVG has an intrinsic width and height of 100%. Since the containing block for the <object> does not have a fixed height, the intrinsic height of  100% cannot be resolved, and it falls back to 150px. (Essentially the height of the containing block and the height of the <object> depend on each other, so something has to give.)

Comment 16

[Daniel Holbert [:dholbert]]

Ok -- from talking to jwatt and fantasai in IRC a bit, I think the correct way to fix this bug is to squash bug 381497.  (see comment 13 -- essentially, when printing, the 100% height on SVG content isn't getting resolved correctly because we're using the wrong containing block.)

Nominating that bug for blocking.

Comment 17

[Daniel Holbert [:dholbert]]

Reassigning to jwatt as he knows our SVG code & SVG expected behavior much better than I do. :)

Note that AFAIK, this bug mostly depends on bug 381497 being fixed, but if we can find a workaround for this in the meantime, that'd definitely be helpful.

Comment 18

[fantasai]

Attachment: patch

This fixes the first two testcases in bug 381479, and also this one. Fixing 381479 properly would probably be pretty difficult and risky and not something we want to tackle for 1.9.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Wouldn't it make more sense to fix ComputeSize() for page frames to just use the right computed height?  Or would that misbehave somehow?

Comment 20

[fantasai]

Attachment: patch v2

(And by 381479 I meant bug 381497.)

Updated patch using ComputeSize(). Thanks for the tip, bz. :)

Comment 21

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 302086 [details] [diff] [review]
patch v2

Looks fine, though it might make sense to store this number in mPD somewhere, since the page nsPageFrame ends up doing the same exact calculation....

Comment 22

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 302086 [details] [diff] [review]
patch v2

>Index: layout/generic/nsPageContentFrame.cpp
>+  nscoord height = (mPD && mPD->mReflowSize.height == NS_UNCONSTRAINEDSIZE)
>+                   ? (NS_UNCONSTRAINEDSIZE)
>+                   : (mPD->mReflowSize.height - 

Er, I just realized there's a weirdness here.  Why null-check mPD?  If you feel that you must, wouldn't it make more sense to do:

  (!mPD || mPD->mReflowSize.height == NS_UNCONSTRAINEDSIZE)

for the conditional?

And remove the parens around NS_UNCONSTRAINEDSIZE after the '?', please.

Comment 23

[fantasai]

Attachment: patch v3

Fixed null-check and added an assert, because I don't think we're supposed to be getting a null mPD here (but I'm not sure).

Comment 24

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 302096 [details] [diff] [review]
patch v3

Looks good.

Comment 25

[Jonathan Watt [:jwatt]]

Excellent. Thanks a lot for taking this fantasai!

Comment 26

[fantasai]

Fix checked in. Thanks, bz.