We need tests for CSS3 Paged Media's page breaks.
Created attachment 58981 [details] add-tests-for-pagebreaks
Created attachment 58984 [details] add-tests-for-pagebreaks-fix-style-and-comments
FYI. The patch is splitted from the following patch, including only newly added tests. https://bugs.webkit.org/show_bug.cgi?id=39735 https://bug-39735-attachments.webkit.org/attachment.cgi?id=57215
Comment on attachment 58984 [details] add-tests-for-pagebreaks-fix-style-and-comments LayoutTests/ChangeLog:5 + Add tests for CSS3 Paged Media's page breaks. Some tests are expected to fail because some features are not implemented. Could you write the names of failing tests? LayoutTests/ChangeLog:5 + Add tests for CSS3 Paged Media's page breaks. Some tests are expected to fail because some features are not implemented. We usually don't write long lines in ChangeLog. Also, it would be better to break line after the bugzilla summary. LayoutTests/printing/script-tests/page-break-after-avoid.js:6 + // block 'page2-1' must move to the next page because it has 'page-break-after:avoid' and 'page2-1' and 'page2-2' cannot be placed in the current page at the same time. We usually don't write long line comments either. Maybe ~100 characters would be the threshold. LayoutTests/ChangeLog:21 + * printing/page-break-mergin-collapsed-expected.txt: Added. s/mergin/margin/ It's somewhat difficult to review 10 tests at once. Could you split this patch? We don't need 10 patches. Maybe 1. for orphans and widows (3 tests) 2. for page-break-*avoid (3 tests) 3. for others (4 tests)
Created attachment 59090 [details] test-for-page-breaks-others
Thank you for the review. (In reply to comment #4) > It's somewhat difficult to review 10 tests at once. Could you split this patch? We don't need 10 patches. Maybe > > 1. for orphans and widows (3 tests) > 2. for page-break-*avoid (3 tests) > 3. for others (4 tests) Sure. I 've split it to three patches: 1. for orphans and widows (3 tests) -> Split to https://bugs.webkit.org/show_bug.cgi?id=40821 2. for page-break-*avoid (3 tests) -> Split to https://bugs.webkit.org/show_bug.cgi?id=40814 3. for others (4 tests) -> Attached to this bug id. I've reflected your comments in each patch. Could you review them?
Comment on attachment 59090 [details] test-for-page-breaks-others WebCore/ChangeLog:23 + git-svn-id: http://svn.webkit.org/repository/webkit/trunk@61310 268f45cc-cd09-0410-ab3c-d52691b4dbfc This ChangeLog looks screwed up. Maybe you didn't mean to include it?
Created attachment 59230 [details] test-for-page-breaks-others-remove-unintentional-chnagelog
Ops. Thank you for pointing out my mistake. It's unintentional. I've remove that WebCore/ChangeLog entry from the patch. Could you review it again? (In reply to comment #7) > (From update of attachment 59090 [details]) > WebCore/ChangeLog:23 > + git-svn-id: http://svn.webkit.org/repository/webkit/trunk@61310 268f45cc-cd09-0410-ab3c-d52691b4dbfc > This ChangeLog looks screwed up. Maybe you didn't mean to include it?
Comment on attachment 59230 [details] test-for-page-breaks-others-remove-unintentional-chnagelog OK
Comment on attachment 59230 [details] test-for-page-breaks-others-remove-unintentional-chnagelog Clearing flags on attachment: 59230 Committed r61530: <http://trac.webkit.org/changeset/61530>
All reviewed patches have been landed. Closing bug.