298283 - implement the 'overflow' property (initial value 'hidden')

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Rob Russell]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050531 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050531 Firefox/1.0+

The test cases
http://www.w3.org/Graphics/SVG/Test/20030813/htmlframe/full-coords-viewattr-01-b.html
and
http://www.w3.org/Graphics/SVG/Test/20030813/htmlframe/full-coords-viewattr-02-b.html
both fail in the lower half, where preserveAspectRatio="slice" is being tested.

Reproducible: Always

Steps to Reproduce:
1. Visit either test url with Firefox Deer Park Alpha 1.1
2. Compare the test SVG image on the left with the reference PNG on the right.


Actual Results:  
The lower tests, with the "slice" value, do not match the reference.

Expected Results:  
The SVG image should match the reference.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Please file any other SVG bugs in Core:SVG instead of Firefox:General.

Comment 2

[Holger Will]

this WORKSFORME, the svg fragments are positioned correctly, "meet" and "slice"
are implemented correctly, the images just dont get cliped.

Comment 3

[Holger Will]

Attachment: testcase

testcase , that shows meet and slice are rendered differently,
just click somewhere in the document to toggle between meet and slice.

Comment 4

[Holger Will]

i forgot to add, it works in
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050616
Firefox/1.0+

Comment 5

[Rob Russell]

Maybe I'm missing something, but isn't clipping important to "slice" ? I see
that the alignment is correct, but the viewBox should be clipped to the
viewport, shouldn't it?

From http://www.w3.org/TR/2002/CR-SVG11-20020430/coords.html#ViewBoxAttribute :
" In this case, if the aspect ratio of the viewBox does not match the viewport,
some of the viewBox will extend beyond the bounds of the viewport (i.e., the
area into which the viewBox will draw is larger than the viewport)."

Or is there a more general bug somewhere about not clipping to the viewport? 

Comment 6

[Holger Will]

(In reply to comment #5)
> Maybe I'm missing something, but isn't clipping important to "slice" ? I see
> that the alignment is correct, but the viewBox should be clipped to the
> viewport, shouldn't it?

i dont know, but it would definitly speed up rendering in most cases, see
https://bugzilla.mozilla.org/show_bug.cgi?id=119217

> 
> Or is there a more general bug somewhere about not clipping to the viewport? 

maybe you can change the subject of this bug to somthing like "mozilla should
allways clip to the viewport". 
though maybe its the same as above mentioned bug.

Comment 7

[Holger Will]

Attachment: new testcase

Comment 8

[Jonathan Watt [:jwatt]]

We don't seem to support the 'overflow' and 'clip' properties as specified here:

  http://www.w3.org/TR/SVG/masking.html#OverflowAndClipProperties

"The initial value for 'overflow' as defined in [CSS2-overflow] is 'visible';
however, SVG's user agent style sheet  overrides this initial value and set the
'overflow' property on elements that establish new viewports (e.g., 'svg'
elements), 'pattern' elements and 'marker' elements to the value 'hidden'."

Comment 9

[Holger Will]

so probably Bug#298282
https://bugzilla.mozilla.org/show_bug.cgi?id=298282
is a dublicate of this bug.

Comment 10

[Boris Zbarsky [:bzbarsky]]

So this is a matter of implementing overflow in general and then adding some
style to svg.css?

Comment 11

[tor]

We do implement overflow for inner svg, and indeed it still seems to work
for simple testcases.  Not sure why these particular examples are failing.

Comment 12

[Holger Will]

Attachment: testcase

another tastcase, showing the real problem, overflow clips to the viewbox, but
should clip to the viewport.

Comment 13

[tor]

Attachment: fix overflow for inner <svg> and <image>

Not sure what that testcase was supposed to display, but this fixes the two w3c
testcases.

Comment 14

[Holger Will]

Attachment: screenshot showing the behavior in deer park(before your patch)

Comment 15

[Holger Will]

Attachment: screenshot of expected behavior

Comment 16

[Holger Will]

i hope this got fixed by your patch as well, maybe its another bug though.

Comment 17

[tor]

Output with the patch is similar to your expected behavior.

Comment 18

[Holger Will]

nice ! thanks for the fix !
how about bug#298282
is that fixed as well ?

Comment 19

[Jonathan Watt [:jwatt]]

Comment on attachment 188045 [details] [diff] [review]
fix overflow for inner <svg> and <image>

(In reply to comment #17)
> Output with the patch is similar to your expected behavior.

Spot on I'd say. The patch looks good too. r=me

>+    nsCOMPtr<nsIDOMSVGMatrix> clipTransform;
>+    if (!mPropagateTransform) {
>+      NS_NewSVGMatrix(getter_AddRefs(clipTransform));
>+    } else {
>+      nsISVGContainerFrame *parent;
>+      CallQueryInterface(mParent, &parent);
>+      if (parent)

I'd quite like an assertion here.

>+        clipTransform = parent->GetCanvasTM();
>     }
>+
>+    if (clipTransform)
>+      canvas->SetClipRect(clipTransform, x, y, width, height);

Comment 20

[tor]

Attachment: also fix <marker>

Comment 21

[Jonathan Watt [:jwatt]]

Comment on attachment 188054 [details] [diff] [review]
also fix <marker>

Cancelling the approval1.8b3 request. Asa says to carry the previous one.

Comment 22

[tor]

*** Bug 298282 has been marked as a duplicate of this bug. ***

Comment 23

[tor]

Checked in.