Bug 96026

Summary: Parcel up logic related to sticky positioning into a Constraints class that will later be used for threaded scrolling
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, eric, gyuyoung.kim, jamesr, rakuco, sam, simon.fraser, skyostil, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch simon.fraser: review+

Simon Fraser (smfr)
Reported 2012-09-06 14:27:09 PDT
Parcel up logic related to sticky positioning into a Constraints class that will later be used for threaded scrolling
Attachments
Patch (25.83 KB, patch)
2012-09-06 15:17 PDT, Simon Fraser (smfr) 		no flags
DetailsFormatted DiffDiff
Patch (27.28 KB, patch)
2012-09-06 16:03 PDT, Simon Fraser (smfr) 		no flags
DetailsFormatted DiffDiff
Patch (27.24 KB, patch)
2012-09-06 16:06 PDT, Simon Fraser (smfr) 		simon.fraser: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2012-09-06 15:17:26 PDT
Created attachment 162602 [details] Patch
James Robinson
Comment 2 2012-09-06 15:42:47 PDT
Comment on attachment 162602 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162602&action=review Looks pretty good. Could you add the new files to the other build systems (evil cackle here). For chromium it's pretty easy, just add them to the big-ass 'webcore_files' list in Source/WebCore/WebCore.gypi > Source/WebCore/ChangeLog:22 > + FixedPositionViewportConstraints is not yet used. If I was writing this, I'd hold off on landing FixedPositionViewportConstraints until it was used and tested. Checked-in-but-dead code has a nasty habit of bitrotting. > Source/WebCore/WebCore.xcodeproj/project.pbxproj:25509 > + 0F605AED15F94848004DF0C0 /* ScrollingConstraints.h in Headers */, should these be sorted?
Simon Fraser (smfr)
Comment 3 2012-09-06 15:56:30 PDT
Comment on attachment 162602 [details] Patch r-, need to add the new files to other build systems
Simon Fraser (smfr)
Comment 4 2012-09-06 16:03:53 PDT
Created attachment 162610 [details] Patch
James Robinson
Comment 5 2012-09-06 16:04:42 PDT
Comment on attachment 162610 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162610&action=review > Source/WebCore/CMakeLists.txt:1735 > + page/scrolling/ScrollingConstraints.cpp > + page/scrolling/ScrollingConstraints.cpp don't need both - looks like convention here is not to list the headers so just nuke one line
James Robinson
Comment 6 2012-09-06 16:05:38 PDT
Comment on attachment 162610 [details] Patch Otherwise looks good, R=me. I'd probably let the EWS bots finish, or land and if you're in a hurry to land watch out for bustages.
Simon Fraser (smfr)
Comment 7 2012-09-06 16:06:08 PDT
Created attachment 162611 [details] Patch
Simon Fraser (smfr)
Comment 8 2012-09-06 16:08:35 PDT
Comment on attachment 162611 [details] Patch Forward James's r+
Simon Fraser (smfr)
Comment 9 2012-09-06 16:20:50 PDT
http://trac.webkit.org/changeset/127795
Note You need to log in before you can comment on or make changes to this bug.