Bug 9753 – SVG with width and height 100% leaves room for scrollbar on the right.

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Joost de Valk (AlthA)]

Testcase says it all, look on the right side of your window.

[Joost de Valk (AlthA)]

Created attachment 9227 [details]
testcase

[Rob Buis]

Created attachment 9296 [details]
First attempt

This patch actually solves a bigger problem, relayouting/resizing of
svg elements that depend on percentages. I will add a testcase
should the code get okayed.
Cheers,

Rob.

[Eric Seidel (no email)]

Comment on attachment 9296 [details]
First attempt

Looks good.

There are a couple mis-placed *'s:
RenderObject *
RenderPath *

Also, I've more commonly seen multi-line ifs broken before the binary operator, instead of after, example:
if (foo
   || bar
   || baz)

Not sure if there is an official practice in that regard or not.

r=me.

[Rob Buis]

Created attachment 9304 [details]
Improved patch+testcase

I fixed MacDome's issues, the || we decided is not crucial since it is done inconsistent
throughout webkit codebase. I also added a testcase based on Joost's one. Note that
all svg tests are changed because all RenderSVGText objects get 15 added to the width
because the scrollbar is not taken into account anymore.
Cheers,

Rob.

[Sam Weinig]

Rob, 
  One other quick style issue is that you have some single line if-statements with curly braces.

[Rob Buis]

Created attachment 9305 [details]
Improved patch+testcase

Fixing Weinigs issues :)
Cheers,

Rob.

[Maciej Stachowiak]

Comment on attachment 9305 [details]
Improved patch+testcase

r=me

Really though I think we ultimately want more of percentage handling to be in the render tree, not the DOM.

[David Kilzer (:ddkilzer)]

Commit r15300 (http://paste.lisp.org/display/22342) appears to have caused the following additional SVG test to fail:

svg/W3C-SVG-1.1/pservers-grad-08-b.svg

[David Kilzer (:ddkilzer)]

(In reply to comment #8)
> Commit r15300 (http://paste.lisp.org/display/22342) appears to have caused the
> following additional SVG test to fail:
> 
> svg/W3C-SVG-1.1/pservers-grad-08-b.svg

This is not related to this bug.  Filed Bug 9834 for it, though.

[Rob Buis]

Created attachment 9346 [details]
Improved patch+testcase

This patch has a better testcase, now it also tests resizing of lines, circles and ellipses, which
proved to have some logical errors in the way they resize using percentages, the improved
testcase should catch this in future.
Sorry for having to clear the review bits again :}
Cheers,

Rob.

[David Kilzer (:ddkilzer)]

Comment on attachment 9296 [details]
First attempt

Clearing review flag.

[David Kilzer (:ddkilzer)]

Comment on attachment 9305 [details]
Improved patch+testcase

Clearing review flag.

[Maciej Stachowiak]

Comment on attachment 9346 [details]
Improved patch+testcase

r=me

[Adele Peterson]

Committed revision 15665.

[David Kilzer (:ddkilzer)]

The patch in Attachment 9346 [details] caused ~190 SVG tests to regress (or need updates to their layout tests).

http://build.webkit.org/changes/3041
http://build.webkit.org/results/post-commit-powerpc-mac-os-x/2851/

[Joost de Valk (AlthA)]

I bet all of those need updated results.

[Adele Peterson]

Comment on attachment 9346 [details]
Improved patch+testcase

R-'ing until we work out the layout test failures

[Rob Buis]

Created attachment 10425 [details]
Another try

This one compiles against ToT. It contains all changed svg testcases, I think they need
careful scrutiny to see whether only some widths changed. I remember something about
these changes hiding some errors already in testcase results, is that still the case or was
that another bug?
Cheers,

Rob.

[Maciej Stachowiak]

Comment on attachment 10425 [details]
Another try

r=me

[Rob Buis]

Landed in r16259.