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

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

Summary: Fix painting and hit testing issues with various pagination and writing mode ...

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	Layout and Rendering (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Dave Hyatt

URL:
Keywords:	InRadar

Depends on:
Blocks:

Reported:	2013-07-18 14:50 PDT by Dave Hyatt
Modified:	2013-07-23 17:01 PDT (History)
CC List:	17 users (show)

See Also:

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 an attachment (proposed patch, testcase, etc.)

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

Description Dave Hyatt 2013-07-18 14:50:35 PDT

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

Comment 1 Dave Hyatt 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.

Comment 2 Dave Hyatt 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?

Comment 3 David Kilzer (:ddkilzer) 2013-07-18 17:23:27 PDT

<rdar://problem/14109351>

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

Created attachment 207097 [details]
Patch that contains code changes

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

Created attachment 207098 [details]
LeftToRight Pagination Mode Tests

Comment 6 WebKit Commit Bot 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.

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

Created attachment 207099 [details]
RightToLeft Layout Tests

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

Created attachment 207100 [details]
TopToBottom Layout Tests

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

Created attachment 207101 [details]
BottomToTop Layout Tests

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

Created attachment 207102 [details]
Other Layout Test changes

Comment 11 Tim Horton 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 #.

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

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

Comment 13 Build Bot 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

Comment 14 Build Bot 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

Comment 15 Build Bot 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

Comment 16 Build Bot 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

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

Created attachment 207106 [details]
Code changes only

Comment 18 Build Bot 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

Comment 19 Build Bot 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

Comment 20 Build Bot 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

Comment 21 Build Bot 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

Comment 22 Build Bot 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

Comment 23 Build Bot 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

Comment 24 Build Bot 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

Comment 25 Build Bot 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

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

Landed in r152911.