Bug 118864

Summary:

Fix painting and hit testing issues with various pagination and writing mode combinations

Product:

WebKit

Reporter:

Dave Hyatt <hyatt>

Component:

Layout and Rendering

Assignee:

Dave Hyatt <hyatt>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

allan.jensen, ap, buildbot, commit-queue, ddkilzer, enrica, esprehn+autocc, esprehn, glenn, kondapallykalyan, macpherson, menard, mitz, rniwa, sam, simon.fraser, thorton

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch that gets all combinations of lr/rl pagination and writing modes working.	none
Patch that contains code changes	none
LeftToRight Pagination Mode Tests	sam: review+, buildbot: commit-queue-
RightToLeft Layout Tests	sam: review+
TopToBottom Layout Tests	sam: review+, buildbot: commit-queue-
BottomToTop Layout Tests	sam: review+
Other Layout Test changes	sam: review+, buildbot: commit-queue-
Code changes only (should pass style queue now)	none
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2	none
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion	none
Code changes only	mitz: review+
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion	none
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion	none
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion	none
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion	none

Dave Hyatt

Reported 2013-07-18 14:50:35 PDT

Fix painting and hit testing issues with various pagination and writing mode combinations

Attachments
Patch that gets all combinations of lr/rl pagination and writing modes working. (24.44 KB, patch) 2013-07-18 15:00 PDT, Dave Hyatt	no flags	Details Formatted Diff Diff
Patch that contains code changes (30.06 KB, patch) 2013-07-19 08:56 PDT, Dave Hyatt	no flags	Details Formatted Diff Diff
LeftToRight Pagination Mode Tests (643.89 KB, patch) 2013-07-19 08:57 PDT, Dave Hyatt	sam: review+ buildbot: commit-queue-	Details Formatted Diff Diff
RightToLeft Layout Tests (640.85 KB, patch) 2013-07-19 08:58 PDT, Dave Hyatt	sam: review+	Details Formatted Diff Diff
TopToBottom Layout Tests (547.10 KB, patch) 2013-07-19 08:59 PDT, Dave Hyatt	sam: review+ buildbot: commit-queue-	Details Formatted Diff Diff
BottomToTop Layout Tests (547.52 KB, patch) 2013-07-19 09:00 PDT, Dave Hyatt	sam: review+	Details Formatted Diff Diff
Other Layout Test changes (18.54 KB, patch) 2013-07-19 09:01 PDT, Dave Hyatt	sam: review+ buildbot: commit-queue-	Details Formatted Diff Diff
Code changes only (should pass style queue now) (30.06 KB, patch) 2013-07-19 09:03 PDT, Dave Hyatt	no flags	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (932.21 KB, application/zip) 2013-07-19 09:24 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (828.43 KB, application/zip) 2013-07-19 09:35 PDT, Build Bot	no flags	Details
Code changes only (29.90 KB, patch) 2013-07-19 09:40 PDT, Dave Hyatt	mitz: review+	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion (738.25 KB, application/zip) 2013-07-19 10:04 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion (719.86 KB, application/zip) 2013-07-19 10:34 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion (772.86 KB, application/zip) 2013-07-19 10:36 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion (914.95 KB, application/zip) 2013-07-19 11:19 PDT, Build Bot	no flags	Details
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Dave Hyatt

Comment 1 2013-07-18 15:00:12 PDT

Created attachment 207025 [details] Patch that gets all combinations of lr/rl pagination and writing modes working. Not ready for review, since I want to do a bunch of cleanup on this to not do the delta computations in every place, etc.

Dave Hyatt

Comment 2 2013-07-18 15:09:34 PDT

Notes to self: (1) Move WebCore::paginationModeForRenderStyle(style) into RenderStyle. (2) Don't need to set the column gap again when the writing mode propagates. (3) Direction propagating doesn't affect column styles so I only needed to check for writing mode changes. (4) Make helper functions to set your initial logical top offset and to compute the block delta. ColumnInfo is a good place for those. (5) Back out Beth and Tim's patch, since it doesn't matter now. (6) Start testing other combinations. Only combinations tested so far are all the lr/rl combinations. That's 4 out of 16, so I need to just test the rest and make sure they all work. (7) Test cases. Need real pagination API test cases. Do layout tests have support for the API?

David Kilzer (:ddkilzer)

Comment 3 2013-07-18 17:23:27 PDT

<rdar://problem/14109351>

Dave Hyatt

Comment 4 2013-07-19 08:56:13 PDT

Created attachment 207097 [details] Patch that contains code changes

Dave Hyatt

Comment 5 2013-07-19 08:57:47 PDT

Created attachment 207098 [details] LeftToRight Pagination Mode Tests

WebKit Commit Bot

Comment 6 2013-07-19 08:57:56 PDT

Attachment 207097 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/css/StyleResolver.cpp', u'Source/WebCore/page/FrameView.cpp', u'Source/WebCore/rendering/RenderBlock.cpp', u'Source/WebCore/rendering/RenderBlock.h', u'Source/WebCore/rendering/RenderBox.cpp', u'Source/WebCore/rendering/RenderLayer.cpp', u'Source/WebCore/rendering/RenderView.cpp', u'Source/WebCore/rendering/RenderView.h', u'Source/WebCore/rendering/style/RenderStyle.cpp', u'Source/WebCore/rendering/style/RenderStyle.h']" exit_code: 1 Source/WebCore/rendering/RenderBlock.cpp:3089: Place brace on its own line for function definitions. [whitespace/braces] [4] Source/WebCore/rendering/RenderBlock.cpp:3101: Place brace on its own line for function definitions. [whitespace/braces] [4] Source/WebCore/rendering/RenderLayer.cpp:4343: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 3 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.

Dave Hyatt

Comment 7 2013-07-19 08:58:48 PDT

Created attachment 207099 [details] RightToLeft Layout Tests

Dave Hyatt

Comment 8 2013-07-19 08:59:44 PDT

Created attachment 207100 [details] TopToBottom Layout Tests

Dave Hyatt

Comment 9 2013-07-19 09:00:29 PDT

Created attachment 207101 [details] BottomToTop Layout Tests

Dave Hyatt

Comment 10 2013-07-19 09:01:27 PDT

Created attachment 207102 [details] Other Layout Test changes

Tim Horton

Comment 11 2013-07-19 09:03:14 PDT

Comment on attachment 207097 [details] Patch that contains code changes View in context: https://bugs.webkit.org/attachment.cgi?id=207097&action=review > Source/WebCore/ChangeLog:17 > + This patch also backs out the maximumScrollPosition change, since it only occurred because Yay! But, please mention the commit #.

Dave Hyatt

Comment 12 2013-07-19 09:03:57 PDT

Created attachment 207103 [details] Code changes only (should pass style queue now)

Build Bot

Comment 13 2013-07-19 09:24:18 PDT

Comment on attachment 207099 [details] RightToLeft Layout Tests Attachment 207099 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1101853 New failing tests: fast/multicol/pagination/RightToLeft-lr.html fast/multicol/pagination/RightToLeft-rl.html

Build Bot

Comment 14 2013-07-19 09:24:20 PDT

Created attachment 207104 [details] Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-12 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3

Build Bot

Comment 15 2013-07-19 09:35:29 PDT

Comment on attachment 207099 [details] RightToLeft Layout Tests Attachment 207099 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1104883 New failing tests: media/video-zoom.html fast/multicol/pagination/RightToLeft-lr.html fast/multicol/pagination/RightToLeft-rl.html

Build Bot

Comment 16 2013-07-19 09:35:33 PDT

Created attachment 207105 [details] Archive of layout-test-results from webkit-ews-07 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.3

Dave Hyatt

Comment 17 2013-07-19 09:40:04 PDT

Created attachment 207106 [details] Code changes only

Build Bot

Comment 18 2013-07-19 10:04:33 PDT

Comment on attachment 207101 [details] BottomToTop Layout Tests Attachment 207101 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1100835 New failing tests: fast/multicol/pagination/BottomToTop-tb.html fast/multicol/pagination/BottomToTop-bt.html

Build Bot

Comment 19 2013-07-19 10:04:36 PDT

Created attachment 207113 [details] Archive of layout-test-results from webkit-ews-08 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-08 Port: mac-mountainlion Platform: Mac OS X 10.8.3

Build Bot

Comment 20 2013-07-19 10:34:31 PDT

Comment on attachment 207098 [details] LeftToRight Pagination Mode Tests Attachment 207098 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1106907 New failing tests: fast/multicol/pagination/LeftToRight-rl.html fast/multicol/pagination/LeftToRight-lr.html

Build Bot

Comment 21 2013-07-19 10:34:34 PDT

Created attachment 207121 [details] Archive of layout-test-results from webkit-ews-05 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-05 Port: mac-mountainlion Platform: Mac OS X 10.8.3

Build Bot

Comment 22 2013-07-19 10:36:30 PDT

Comment on attachment 207100 [details] TopToBottom Layout Tests Attachment 207100 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1104896 New failing tests: fast/multicol/pagination/TopToBottom-bt.html fast/multicol/pagination/TopToBottom-tb.html

Build Bot

Comment 23 2013-07-19 10:36:32 PDT

Created attachment 207123 [details] Archive of layout-test-results from webkit-ews-01 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.3

Build Bot

Comment 24 2013-07-19 11:19:49 PDT

Comment on attachment 207102 [details] Other Layout Test changes Attachment 207102 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1096904 New failing tests: fast/multicol/shrink-to-column-height-for-pagination.html svg/batik/text/smallFonts.svg

Build Bot

Comment 25 2013-07-19 11:19:52 PDT

Created attachment 207128 [details] Archive of layout-test-results from webkit-ews-01 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.3

Dave Hyatt

Comment 26 2013-07-19 11:35:50 PDT

Landed in r152911.

Note You need to log in before you can comment on or make changes to this bug.