Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(492)

Issue 2021703002: Display table header groups at the top of each page (Closed)

Created:
4 years, 6 months ago by rhogan
Modified:
4 years, 6 months ago
CC:
blink-reviews, blink-reviews-layout_chromium.org, blink-reviews-paint_chromium.org, chromium-reviews, dshwang, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, slimming-paint-reviews_chromium.org, szager+layoutwatch_chromium.org, zoltan1
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Display table header groups at the top of each page FF, IE and Edge all repeat a table header group at the top of each printed page. This area is underspecified but after some discussion in the bug we agreed to repeat the header group if it has break-inside:avoid. We make this the default style for theads when printing. BUG=24826 Committed: https://crrev.com/81c0fc6d4e08e4e2bb4eb8a42747f21b13303616 Cr-Commit-Position: refs/heads/master@{#397915}

Patch Set 1 #

Patch Set 2 : Updated #

Patch Set 3 : Updated #

Total comments: 8

Patch Set 4 : Updated #

Total comments: 7

Patch Set 5 : Updated #

Total comments: 3

Patch Set 6 : Updated #

Patch Set 7 : Updated #

Patch Set 8 : Updated #

Patch Set 9 : Updated #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1043 lines, -2 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 3 4 5 6 7 8 1 chunk +3 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fragmentation/single-line-cells-repeating-thead.html View 1 chunk +50 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fragmentation/single-line-cells-repeating-thead-break-inside-on-thead-only.html View 1 chunk +50 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fragmentation/single-line-cells-repeating-thead-break-inside-on-thead-only-expected.html View 1 chunk +50 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fragmentation/single-line-cells-repeating-thead-expected.html View 1 chunk +50 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fragmentation/single-line-cells-repeating-thead-starts-middle-of-page.html View 1 chunk +53 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fragmentation/single-line-cells-repeating-thead-starts-middle-of-page-expected.html View 1 chunk +53 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fragmentation/single-line-cells-self-painting-thead-break-inside-on-thead-only.html View 1 2 3 1 chunk +51 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fragmentation/single-line-cells-self-painting-thead-break-inside-on-thead-only-expected.html View 1 2 3 1 chunk +50 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/platform/linux/printing/thead-repeats-at-top-of-each-page-expected.png View 1 2 Binary file 0 comments Download
A third_party/WebKit/LayoutTests/printing/thead-repeats-at-top-of-each-page.html View 1 2 1 chunk +198 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/printing/thead-repeats-at-top-of-each-page-expected.txt View 1 2 1 chunk +368 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/html.css View 1 2 3 4 5 6 1 chunk +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBox.cpp View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutTableSection.cpp View 1 2 3 4 5 6 7 2 chunks +9 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/paint/TableSectionPainter.h View 1 2 3 4 5 6 7 1 chunk +8 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/paint/TableSectionPainter.cpp View 1 2 3 4 5 6 7 2 chunks +44 lines, -0 lines 0 comments Download

Messages

Total messages: 24 (7 generated)
rhogan
4 years, 6 months ago (2016-05-29 12:20:14 UTC) #3
rhogan
@wangxianzhu: can you take a look at what I'm doing with the painting? Does it ...
4 years, 6 months ago (2016-05-30 18:12:30 UTC) #4
Xianzhu
I can review the paint/ part. I'm not familiar with pagination strut in LayoutTableSection. https://codereview.chromium.org/2021703002/diff/40001/third_party/WebKit/Source/core/paint/TablePainter.cpp ...
4 years, 6 months ago (2016-05-31 02:11:34 UTC) #5
eae
LGTM for layout/
4 years, 6 months ago (2016-05-31 18:00:08 UTC) #6
rhogan
https://codereview.chromium.org/2021703002/diff/40001/third_party/WebKit/Source/core/paint/TablePainter.cpp File third_party/WebKit/Source/core/paint/TablePainter.cpp (right): https://codereview.chromium.org/2021703002/diff/40001/third_party/WebKit/Source/core/paint/TablePainter.cpp#newcode44 third_party/WebKit/Source/core/paint/TablePainter.cpp:44: } On 2016/05/31 at 02:11:34, Xianzhu wrote: > We ...
4 years, 6 months ago (2016-06-01 18:39:46 UTC) #7
Xianzhu
https://codereview.chromium.org/2021703002/diff/40001/third_party/WebKit/Source/core/paint/TablePainter.cpp File third_party/WebKit/Source/core/paint/TablePainter.cpp (right): https://codereview.chromium.org/2021703002/diff/40001/third_party/WebKit/Source/core/paint/TablePainter.cpp#newcode21 third_party/WebKit/Source/core/paint/TablePainter.cpp:21: void TablePainter::paintRepeatingHeaderGroup(const PaintInfo& paintInfo, const LayoutPoint& paintOffset, LayoutBox* child, ...
4 years, 6 months ago (2016-06-01 19:19:39 UTC) #8
rhogan
On 2016/06/01 at 19:19:39, wangxianzhu wrote: > > However, to make the code not strictly ...
4 years, 6 months ago (2016-06-02 21:42:50 UTC) #9
Xianzhu
https://codereview.chromium.org/2021703002/diff/60001/third_party/WebKit/Source/core/paint/TableSectionPainter.cpp File third_party/WebKit/Source/core/paint/TableSectionPainter.cpp (right): https://codereview.chromium.org/2021703002/diff/60001/third_party/WebKit/Source/core/paint/TableSectionPainter.cpp#newcode42 third_party/WebKit/Source/core/paint/TableSectionPainter.cpp:42: // TODO(rhogan): We don't paint a header repeatedly if ...
4 years, 6 months ago (2016-06-02 23:03:53 UTC) #10
rhogan
On 2016/06/02 at 23:03:53, wangxianzhu wrote: Applied all your comments - very nice feedback!
4 years, 6 months ago (2016-06-03 18:35:03 UTC) #11
rhogan
On 2016/06/03 at 18:35:03, rhogan wrote: > On 2016/06/02 at 23:03:53, wangxianzhu wrote: > > ...
4 years, 6 months ago (2016-06-03 18:36:55 UTC) #12
Xianzhu
paint/ lgtm with several comments. https://codereview.chromium.org/2021703002/diff/80001/third_party/WebKit/Source/core/paint/TableSectionPainter.cpp File third_party/WebKit/Source/core/paint/TableSectionPainter.cpp (right): https://codereview.chromium.org/2021703002/diff/80001/third_party/WebKit/Source/core/paint/TableSectionPainter.cpp#newcode59 third_party/WebKit/Source/core/paint/TableSectionPainter.cpp:59: paintCollapsedSectionBorders(paintInfo.forDescendants(), paginationOffset, currentBorderValue); Remove ...
4 years, 6 months ago (2016-06-03 18:45:41 UTC) #13
eae
OK LGTM
4 years, 6 months ago (2016-06-03 20:13:32 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2021703002/150001
4 years, 6 months ago (2016-06-04 10:30:10 UTC) #17
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/82094)
4 years, 6 months ago (2016-06-04 11:40:09 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2021703002/150001
4 years, 6 months ago (2016-06-04 11:50:21 UTC) #21
commit-bot: I haz the power
Committed patchset #9 (id:150001)
4 years, 6 months ago (2016-06-04 12:14:09 UTC) #22
commit-bot: I haz the power
4 years, 6 months ago (2016-06-04 12:15:43 UTC) #24
Message was sent while issue was closed.
Patchset 9 (id:??) landed as
https://crrev.com/81c0fc6d4e08e4e2bb4eb8a42747f21b13303616
Cr-Commit-Position: refs/heads/master@{#397915}

Powered by Google App Engine
This is Rietveld 408576698