Bug 61860

Summary: Remove duplicated code in various computeReplacedLogical*() functions
Product: WebKit Reporter: Nikolas Zimmermann <zimmermann>
Component: Layout and RenderingAssignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, krit, rwlbuis
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch v2 rwlbuis: review+

Nikolas Zimmermann
Reported 2011-06-01 06:54:43 PDT
Remove duplicated code in various computeReplacedLogical*() functions. In the recent RenderPart work (bug 10526) we noticed that some code is duplicated, fix that.
Attachments
Patch (15.64 KB, patch)
2011-06-01 07:00 PDT, Nikolas Zimmermann 		no flags
DetailsFormatted DiffDiff
Patch v2 (15.64 KB, patch)
2011-06-01 07:02 PDT, Nikolas Zimmermann 		rwlbuis: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Nikolas Zimmermann
Comment 1 2011-06-01 07:00:12 PDT
Created attachment 95595 [details] Patch
Nikolas Zimmermann
Comment 2 2011-06-01 07:02:25 PDT
Created attachment 95597 [details] Patch v2 Oops, uploaded the wrong patch.
Rob Buis
Comment 3 2011-06-01 07:08:41 PDT
Comment on attachment 95597 [details] Patch v2 LGTM
Nikolas Zimmermann
Comment 4 2011-06-01 07:21:33 PDT
Landed in r87801.
Eric Seidel (no email)
Comment 5 2011-06-01 08:55:26 PDT
Comment on attachment 95597 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=95597&action=review > Source/WebCore/rendering/RenderPart.h:53 > + int computeEmbeddedDocumentReplacedWidth(RenderSVGRoot* contentRenderer, bool includeMaxWidth) const; > + int computeEmbeddedDocumentReplacedHeight(RenderSVGRoot* contentRenderer) const; Seems odd to make RenderPart depend on REnderSVGRoot for these generic sounding functions. I guess you plan to fix that later?
Nikolas Zimmermann
Comment 6 2011-06-01 09:08:12 PDT
(In reply to comment #5) > (From update of attachment 95597 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=95597&action=review > > > Source/WebCore/rendering/RenderPart.h:53 > > + int computeEmbeddedDocumentReplacedWidth(RenderSVGRoot* contentRenderer, bool includeMaxWidth) const; > > + int computeEmbeddedDocumentReplacedHeight(RenderSVGRoot* contentRenderer) const; > > Seems odd to make RenderPart depend on REnderSVGRoot for these generic sounding functions. I guess you plan to fix that later? This is completely correct, all code related to size-negotiation will be generalized into RenderReplaced, which is the base for RenderImage/RenderPart (through RenderWidget). The current RenderPart stuff for embedding SVG documents covers size-negotiation for object/embed and iframe. RenderImage still contains its own code for size-negotiation which is broken in some cases (SVG as image problems with relative sizes etc.) - that code shall die and join with the new code in RenderPart, which will then be refactored into RenderReplaced. I'm working on this as we speak.
Note You need to log in before you can comment on or make changes to this bug.