Bug 9753 - SVG with width and height 100% leaves room for scrollbar on the right.
Summary: SVG with width and height 100% leaves room for scrollbar on the right.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords: HasReduction
Depends on:
Blocks:
 
Reported: 2006-07-06 04:52 PDT by Joost de Valk (AlthA)
Modified: 2006-09-07 02:30 PDT (History)
4 users (show)

See Also:


Attachments
testcase (108 bytes, application/xml)
2006-07-06 04:52 PDT, Joost de Valk (AlthA)
no flags Details
First attempt (9.44 KB, patch)
2006-07-09 07:28 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Improved patch+testcase (10.79 KB, patch)
2006-07-09 12:32 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Improved patch+testcase (10.74 KB, patch)
2006-07-09 13:51 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Improved patch+testcase (14.15 KB, patch)
2006-07-10 13:13 PDT, Rob Buis
adele: review-
Details | Formatted Diff | Diff
Another try (737.87 KB, patch)
2006-09-06 13:30 PDT, Rob Buis
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joost de Valk (AlthA) 2006-07-06 04:52:24 PDT
Testcase says it all, look on the right side of your window.
Comment 1 Joost de Valk (AlthA) 2006-07-06 04:52:50 PDT
Created attachment 9227 [details]
testcase
Comment 2 Rob Buis 2006-07-09 07:28:14 PDT
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.
Comment 3 Eric Seidel (no email) 2006-07-09 07:46:40 PDT
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.
Comment 4 Rob Buis 2006-07-09 12:32:52 PDT
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.
Comment 5 Sam Weinig 2006-07-09 13:07:24 PDT
Rob, 
  One other quick style issue is that you have some single line if-statements with curly braces.
Comment 6 Rob Buis 2006-07-09 13:51:59 PDT
Created attachment 9305 [details]
Improved patch+testcase

Fixing Weinigs issues :)
Cheers,

Rob.
Comment 7 Maciej Stachowiak 2006-07-10 00:44:40 PDT
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.
Comment 8 David Kilzer (:ddkilzer) 2006-07-10 07:12:40 PDT
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

Comment 9 David Kilzer (:ddkilzer) 2006-07-10 07:58:34 PDT
(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.

Comment 10 Rob Buis 2006-07-10 13:13:55 PDT
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.
Comment 11 David Kilzer (:ddkilzer) 2006-07-12 07:38:35 PDT
Comment on attachment 9296 [details]
First attempt

Clearing review flag.
Comment 12 David Kilzer (:ddkilzer) 2006-07-12 07:41:29 PDT
Comment on attachment 9305 [details]
Improved patch+testcase

Clearing review flag.
Comment 13 Maciej Stachowiak 2006-07-13 00:39:32 PDT
Comment on attachment 9346 [details]
Improved patch+testcase

r=me
Comment 14 Adele Peterson 2006-07-27 21:47:14 PDT
Committed revision 15665.
Comment 15 David Kilzer (:ddkilzer) 2006-07-28 03:27:00 PDT
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/
Comment 16 Joost de Valk (AlthA) 2006-07-28 04:41:41 PDT
I bet all of those need updated results.
Comment 17 Adele Peterson 2006-07-28 11:55:54 PDT
Comment on attachment 9346 [details]
Improved patch+testcase

R-'ing until we work out the layout test failures
Comment 18 Rob Buis 2006-09-06 13:30:24 PDT
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.
Comment 19 Maciej Stachowiak 2006-09-06 13:36:13 PDT
Comment on attachment 10425 [details]
Another try

r=me
Comment 20 Rob Buis 2006-09-07 02:30:50 PDT
Landed in r16259.