Bug 61860 - Remove duplicated code in various computeReplacedLogical*() functions
Summary: Remove duplicated code in various computeReplacedLogical*() functions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-01 06:54 PDT by Nikolas Zimmermann
Modified: 2011-06-01 09:08 PDT (History)
3 users (show)

See Also:


Attachments
Patch (15.64 KB, patch)
2011-06-01 07:00 PDT, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Patch v2 (15.64 KB, patch)
2011-06-01 07:02 PDT, Nikolas Zimmermann
rwlbuis: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 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.
Comment 1 Nikolas Zimmermann 2011-06-01 07:00:12 PDT
Created attachment 95595 [details]
Patch
Comment 2 Nikolas Zimmermann 2011-06-01 07:02:25 PDT
Created attachment 95597 [details]
Patch v2

Oops, uploaded the wrong patch.
Comment 3 Rob Buis 2011-06-01 07:08:41 PDT
Comment on attachment 95597 [details]
Patch v2

LGTM
Comment 4 Nikolas Zimmermann 2011-06-01 07:21:33 PDT
Landed in r87801.
Comment 5 Eric Seidel (no email) 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?
Comment 6 Nikolas Zimmermann 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.