WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
12936
Fix most remaining <use> bugs.
https://bugs.webkit.org/show_bug.cgi?id=12936
Summary
Fix most remaining <use> bugs.
Nikolas Zimmermann
Reported
2007-03-01 10:04:10 PST
Filing one master bug, whose attached patch fixes several bugs: 12926, 12267, 12916, 12917, 12838 Attaching patch soon.
Attachments
Initial patch
(24.10 KB, patch)
2007-03-01 10:16 PST
,
Nikolas Zimmermann
darin
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Nikolas Zimmermann
Comment 1
2007-03-01 10:05:17 PST
***
Bug 12926
has been marked as a duplicate of this bug. ***
Nikolas Zimmermann
Comment 2
2007-03-01 10:05:43 PST
***
Bug 12267
has been marked as a duplicate of this bug. ***
Nikolas Zimmermann
Comment 3
2007-03-01 10:06:27 PST
***
Bug 12916
has been marked as a duplicate of this bug. ***
Nikolas Zimmermann
Comment 4
2007-03-01 10:06:48 PST
***
Bug 12917
has been marked as a duplicate of this bug. ***
Nikolas Zimmermann
Comment 5
2007-03-01 10:07:33 PST
***
Bug 12838
has been marked as a duplicate of this bug. ***
Nikolas Zimmermann
Comment 6
2007-03-01 10:16:45 PST
Created
attachment 13437
[details]
Initial patch Doesn't contain any layout test updates yet - as I'm on a "--svg-experimental" build. If this patch gets r+ I'll create new test results with a fresh ToT build + my patch applied.
Darin Adler
Comment 7
2007-03-04 23:08:34 PST
Comment on
attachment 13437
[details]
Initial patch + return svgElement; Avoid reference count churn by saying return svgElement.release(), which allows the RefPtr inside the function to give up the pointer without doing a ref followed by a deref. + String widthString = String::number(width().value()); + String heightString = String::number(height().value()); A shame to compute these both even when there's no width or height attribute. I sugestmoving these expressions inside the if statements. You probably don't need the local variables. + String transformString = String::format("translate(%f, %f)", use->x().value(), use->y().value()); I see this pattern repeated multiple times. I suggest some sort of helper function to do this. Maybe a new function appendTransform(const FloatSize&) as a member or just a helper function. Do the two layout tests really cover all the problems? r=me
Nikolas Zimmermann
Comment 8
2007-03-05 17:49:04 PST
Landed in
r19972
.
Nikolas Zimmermann
Comment 9
2007-03-05 17:50:54 PST
Hi Darin, forgot to fix the issue you mentioned before comitting. Need to get some sleep now will fix it tomorrow.
> (From update of
attachment 13437
[details]
[edit]) > + return svgElement; > > Avoid reference count churn by saying return svgElement.release(), which allows > the RefPtr inside the function to give up the pointer without doing a ref > followed by a deref.
okay
> > + String widthString = String::number(width().value()); > + String heightString = String::number(height().value()); > > A shame to compute these both even when there's no width or height attribute. I > sugestmoving these expressions inside the if statements. You probably don't > need the local variables.
Okay.
> > + String transformString = String::format("translate(%f, > %f)", use->x().value(), use->y().value()); > > I see this pattern repeated multiple times. I suggest some sort of helper > function to do this. Maybe a new function appendTransform(const FloatSize&) as > a member or just a helper function.
Sounds reasonable.
> Do the two layout tests really cover all the problems?
Yes, I think so - will need to check all individual bug reports again. Niko
jay
Comment 10
2007-03-13 02:41:23 PDT
niko, reopening 12838 as broken, also widening scope as CSS styles are effected irrespective of scripting.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug