WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 31393
externalRepresentation should take Frame as the argument
https://bugs.webkit.org/show_bug.cgi?id=31393
Summary
externalRepresentation should take Frame as the argument
Shinichiro Hamaji
Reported
2009-11-11 23:57:15 PST
As suggested in
Bug 30555
, WebCore::externalRepresentation should take Frame* instead of RenderObject*.
Attachments
Patch v1
(6.81 KB, patch)
2009-11-12 00:00 PST
,
Shinichiro Hamaji
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Shinichiro Hamaji
Comment 1
2009-11-12 00:00:41 PST
Created
attachment 43036
[details]
Patch v1
Darin Adler
Comment 2
2009-11-12 09:22:44 PST
Comment on
attachment 43036
[details]
Patch v1 This is a good change. But you're missing an opportunity to fix a major bug in externalRepresentation. The layout needs to be moved to the top of the function, before the SVG call to writeRenderResources. And the RenderObject should be fetched only after the call to layout. I'll say r=me assuming that you'll do those fixes in a subsequent patch, because those are really the motivation for making this change.
Shinichiro Hamaji
Comment 3
2009-11-12 18:37:27 PST
Comment on
attachment 43036
[details]
Patch v1 Clearing flags on attachment: 43036 Committed
r50923
: <
http://trac.webkit.org/changeset/50923
>
Shinichiro Hamaji
Comment 4
2009-11-12 18:37:36 PST
All reviewed patches have been landed. Closing bug.
Shinichiro Hamaji
Comment 5
2009-11-12 18:49:22 PST
Yeah, I'll fix it, but I wanted to avoid changing both WebCore interface and WebCore implementation in one patch. An interface change requires change of multiple platform code, which has the risk of build breakage. So, I want to keep the risk as possible. I'll post another patch after I confirm this change doesn't break anything in build bots. I think I should have described this plan, sorry. Thanks for the review.
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