| Summary: | SVG with width and height 100% leaves room for scrollbar on the right. | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Joost de Valk (AlthA) <joost> | ||||||||||||||
| Component: | SVG | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | adele, ddkilzer, mjs, rwlbuis | ||||||||||||||
| Priority: | P2 | Keywords: | HasReduction | ||||||||||||||
| Version: | 420+ | ||||||||||||||||
| Hardware: | Mac | ||||||||||||||||
| OS: | OS X 10.4 | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Joost de Valk (AlthA)
2006-07-06 04:52:24 PDT
Created attachment 9227 [details]
testcase
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 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.
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.
Rob, One other quick style issue is that you have some single line if-statements with curly braces. Created attachment 9305 [details]
Improved patch+testcase
Fixing Weinigs issues :)
Cheers,
Rob.
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.
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 (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. 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 on attachment 9296 [details]
First attempt
Clearing review flag.
Comment on attachment 9305 [details]
Improved patch+testcase
Clearing review flag.
Comment on attachment 9346 [details]
Improved patch+testcase
r=me
Committed revision 15665. 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/ I bet all of those need updated results. Comment on attachment 9346 [details]
Improved patch+testcase
R-'ing until we work out the layout test failures
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 on attachment 10425 [details]
Another try
r=me
Landed in r16259. |