Bug 122959

Summary: [CSSRegions] Move regions auto-size code into RenderNamedFlowFragment
Product: WebKit Reporter: Mihnea Ovidenie <mihnea>
Component: CSSAssignee: Andrei Bucur <abucur>
Status: RESOLVED FIXED    
Severity: Normal CC: abucur, commit-queue, esprehn+autocc, glenn, kondapallykalyan, mibalan, WebkitBugTracker
Priority: P2 Keywords: AdobeTracked
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 57312    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Mihnea Ovidenie
Reported 2013-10-17 05:28:26 PDT
After https://bugs.webkit.org/show_bug.cgi?id=119135, the region auto-size code should be moved from RenderRegion class to the new RenderNamedFlowFragment class. RenderNamedFlowFragment will contain functionality specific to CSSRegions only, as it is the case for region auto-size.
Attachments
Patch (31.38 KB, patch)
2014-01-08 07:59 PST, Andrei Bucur 		no flags
DetailsFormatted DiffDiff
Patch (29.58 KB, patch)
2014-01-09 02:04 PST, Andrei Bucur 		no flags
DetailsFormatted DiffDiff
Patch (28.71 KB, patch)
2014-01-09 03:26 PST, Andrei Bucur 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Andrei Bucur
Comment 1 2014-01-08 07:59:23 PST
Created attachment 220633 [details] Patch
Mihnea Ovidenie
Comment 2 2014-01-08 08:23:47 PST
(In reply to comment #1) > Created an attachment (id=220633) [details] > Patch Would not have been easier to make *hasAutoLogicalHeight* a virtual which returns false in RenderRegion and do the check in RenderNamedFlowFragment? The static implementation in RenderFlowThread calls isRenderNamedFlowFragment() which is already a virtual method.
Andrei Bucur
Comment 3 2014-01-09 02:04:09 PST
Created attachment 220704 [details] Patch
Mihnea Ovidenie
Comment 4 2014-01-09 02:15:45 PST
Comment on attachment 220704 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220704&action=review r=me, please take a look at the comments before landing. > Source/WebCore/rendering/RenderNamedFlowFragment.cpp:159 > + ASSERT(newLogicalHeight < LayoutUnit::max() / 2); I would use RenderFlowThread::maxLogicalHeight() instead of LayoutUnit::max() / 2. > Source/WebCore/rendering/RenderTreeAsText.cpp:671 > + if (isRenderNamedFlowFragment && toRenderNamedFlowFragment(renderRegion)->hasAutoLogicalHeight()) Since hasAutoLogicalHeight always returns false for RenderRegion, you can still use the previous condition.
Andrei Bucur
Comment 5 2014-01-09 03:26:22 PST
Created attachment 220709 [details] Patch
WebKit Commit Bot
Comment 6 2014-01-09 04:02:22 PST
Comment on attachment 220709 [details] Patch Clearing flags on attachment: 220709 Committed r161553: <http://trac.webkit.org/changeset/161553>
WebKit Commit Bot
Comment 7 2014-01-09 04:02:25 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.