[root layer scrolls] Fix printing [chromium/src : master]
Stefan Zager (Gerrit)
Stefan Zager restored this change.
To view, visit change 729713. To unsubscribe, or for help writing mail filters, visit settings.
Stefan Zager (Gerrit)
This covers almost all of the printing tests. Sorry, I didn't do the thing where I first copy the old baselines into the flag-specific directory. For this many tests, it would have been a major PITA.
Philip Rogers (Gerrit)
Nice!
4 comments:
File third_party/WebKit/LayoutTests/FlagExpectations/root-layer-scrolls:
Patch Set #7, Line 287:
crbug.com/417782 printing/subframes-percentage-height.html [ Failure ]Does this test just need a rebaseline?
Patch Set #7, Line 288:
crbug.com/417782 printing/thead-repeats-at-top-of-each-page.html [ Failure ]Does this test just need a rebaseline?
File third_party/WebKit/Source/core/paint/FramePainter.cpp:
Patch Set #7, Line 194:
layer_painter.Paint(context, LayoutRect(rect), local_paint_flags,Optional: It's confusing that local_paint_flags becomes the global paint flags and root_layer_paint_flags becomes the local paint flags. WYDT of renaming "local_paint_flags" to "updated_global_paint_flags"?
File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:
Patch Set #7, Line 1322:
needs_horizontal_scrollbar = needs_vertical_scrollbar = false;A few lines below we will add these back if the scrollbar mode is kScrollbarAlwaysOn. Should that happen?
To view, visit change 729713. To unsubscribe, or for help writing mail filters, visit settings.
Stefan Zager (Gerrit)
4 comments:
Patch Set #7, Line 287:
crbug.com/417782 printing/subframes-percentage-height.html [ Failure ]Does this test just need a rebaseline?
No, this one is still broken.
Patch Set #7, Line 288:
crbug.com/417782 printing/thead-repeats-at-top-of-each-page.html [ Failure ]Does this test just need a rebaseline?
No, this one is still broken.
File third_party/WebKit/Source/core/paint/FramePainter.cpp:
Patch Set #7, Line 194:
layer_painter.Paint(context, LayoutRect(rect), updated_global_paint_flags,Optional: It's confusing that local_paint_flags becomes the global paint flags and root_layer_paint_ […]
Done
File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:
Patch Set #7, Line 1322:
needs_vertical_scrollbar = Box().ScrollsOverflowY();A few lines below we will add these back if the scrollbar mode is kScrollbarAlwaysOn. […]
I don't know for sure, but I guess probably not. I moved the printing check to LayoutView::CalculateScrollbarModes, which I think is where it belongs (and will fix the issue you point out).
To view, visit change 729713. To unsubscribe, or for help writing mail filters, visit settings.
Philip Rogers (Gerrit)
LGTM. Please wait for Steve's review too.
Patch set 8:Code-Review +1
Steve Kobes (Gerrit)
Did you check all the baselines manually, or are they verified by pattern matching like the other big rebaseline?
1 comment:
File third_party/WebKit/Source/core/layout/LayoutView.cpp:
Patch Set #8, Line 679:
scrollbarsIs this also true for iframes?
To view, visit change 729713. To unsubscribe, or for help writing mail filters, visit settings.
Stefan Zager (Gerrit)
Patch Set 8:
(1 comment)
Did you check all the baselines manually, or are they verified by pattern matching like the other big rebaseline?
I checked them manually from:
http://wince.sfo.corp.google.com:8766/rls/
1 comment:
Patch Set #8, Line 679:
scrollbarsIs this also true for iframes?
Yes, I verified it by looking at the (non-RLS) print preview of this:
<iframe srcdoc='<div style="font-size: 144px">Hello, world!</div>'></iframe>
<div style='height:2000px'></div>
To view, visit change 729713. To unsubscribe, or for help writing mail filters, visit settings.
Philip Rogers (Gerrit)
1 comment:
Patch Set #8, Line 679:
scrollbars
Yes, I verified it by looking at the (non-RLS) print preview of this: […]
Is that correct? It looks like we don't print the scrollbars but I thought we said that we should print them. I don't think we need to fix that now, but maybe file a bug? Alternatively, could this be if (is-main-frame && document.Printing()) { ...
To view, visit change 729713. To unsubscribe, or for help writing mail filters, visit settings.
Stefan Zager (Gerrit)
1 comment:
Patch Set #8, Line 679:
scrollbars
Is that correct? It looks like we don't print the scrollbars but I thought we said that we should pr […]
Actually, what I'm seeing is that space is reserved for the iframe's scrollbars, but they aren't printed, which looks kinda weird:
https://screenshot.googleplex.com/K1u2QEUzYsK
This patch actually fixes that issue; here's the new (non-RLS) print preview:
https://screenshot.googleplex.com/8g2jFmDkgaD
(The resolution of the fixed one is different because I took the screen shot of chrome running inside remote desktop).
To view, visit change 729713. To unsubscribe, or for help writing mail filters, visit settings.
Philip Rogers (Gerrit)
Patch Set 8:
(1 comment)
The printing of white space definitely looks bad. What I'm wondering is whether we should be printing non-frame scrollbars. When we chatted about this over lunch, I thought you successfully argued for painting non-frame scrollbars because it explains that content is shifted and by how much.
Stefan Zager (Gerrit)
Patch Set 8:
Patch Set 8:
(1 comment)
The printing of white space definitely looks bad. What I'm wondering is whether we should be printing non-frame scrollbars. When we chatted about this over lunch, I thought you successfully argued for painting non-frame scrollbars because it explains that content is shifted and by how much.
That's as may be, but I don't intend to fix it with this patch.
Philip Rogers (Gerrit)
Patch Set 8:
Patch Set 8:
Patch Set 8:
(1 comment)
The printing of white space definitely looks bad. What I'm wondering is whether we should be printing non-frame scrollbars. When we chatted about this over lunch, I thought you successfully argued for painting non-frame scrollbars because it explains that content is shifted and by how much.
That's as may be, but I don't intend to fix it with this patch.
Just wondering if you could file I bug and maybe put a todo here. I think you're making the right choice not to fix that in this patch.
Stefan Zager (Gerrit)
Patch Set 8:
Patch Set 8:
Patch Set 8:
Patch Set 8:
(1 comment)
The printing of white space definitely looks bad. What I'm wondering is whether we should be printing non-frame scrollbars. When we chatted about this over lunch, I thought you successfully argued for painting non-frame scrollbars because it explains that content is shifted and by how much.
That's as may be, but I don't intend to fix it with this patch.
Just wondering if you could file I bug and maybe put a todo here. I think you're making the right choice not to fix that in this patch.
Done.
Philip Rogers (Gerrit)
Patch Set 8:
Patch Set 8:
Patch Set 8:
Patch Set 8:
Patch Set 8:
(1 comment)
The printing of white space definitely looks bad. What I'm wondering is whether we should be printing non-frame scrollbars. When we chatted about this over lunch, I thought you successfully argued for painting non-frame scrollbars because it explains that content is shifted and by how much.
That's as may be, but I don't intend to fix it with this patch.
Just wondering if you could file I bug and maybe put a todo here. I think you're making the right choice not to fix that in this patch.
Done.
Thanks, still LGTM
Steve Kobes (Gerrit)
Patch set 9:Code-Review +1
Stefan Zager (Gerrit)
Patch set 9:Commit-Queue +2
Commit Bot (Gerrit)
Try jobs failed on following builders:
linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/535179)
Stefan Zager (Gerrit)
Patch set 9:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot merged this change.
[root layer scrolls] Fix printing
When printing, the root layer should not have scrollbars, and it should
not clip its overflow.
Bug: 774716,774716
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I4d5ecd8cf944a5870568dd08d5a6081bce2b5900
Reviewed-on: https://chromium-review.googlesource.com/729713
Reviewed-by: Steve Kobes <sko...@chromium.org>
Reviewed-by: Philip Rogers <p...@chromium.org>
Commit-Queue: Stefan Zager <sza...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510944}
---
M third_party/WebKit/LayoutTests/FlagExpectations/root-layer-scrolls
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/paint/printing/print-box-shadow-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/paint/printing/print-text-shadow-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/printing/absolute-position-headers-and-footers-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/printing/ellipsis-printing-style-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/printing/fixed-positioned-but-static-headers-and-footers-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/printing/fixed-positioned-headers-and-footers-absolute-covering-some-pages-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/printing/fixed-positioned-headers-and-footers-clipped-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/printing/fixed-positioned-headers-and-footers-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/printing/fixed-positioned-headers-and-footers-inside-transform-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/printing/fixed-positioned-headers-and-footers-larger-than-page-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/printing/forced-break-tree-dump-only-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/printing/iframe-print-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/printing/list-item-with-empty-first-line-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/printing/tfoot-repeats-at-bottom-of-each-page-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/printing/tfoot-repeats-at-bottom-of-each-page-multiple-tables-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/linux/printing/thead-repeats-at-top-of-each-page-multiple-tables-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/paint/printing/print-text-shadow-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/printing/absolute-position-headers-and-footers-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/printing/ellipsis-printing-style-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/printing/fixed-positioned-but-static-headers-and-footers-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/printing/fixed-positioned-headers-and-footers-absolute-covering-some-pages-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/printing/fixed-positioned-headers-and-footers-clipped-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/printing/fixed-positioned-headers-and-footers-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/printing/fixed-positioned-headers-and-footers-inside-transform-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/printing/fixed-positioned-headers-and-footers-larger-than-page-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/printing/forced-break-tree-dump-only-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/printing/iframe-print-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/printing/list-item-with-empty-first-line-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/printing/tfoot-repeats-at-bottom-of-each-page-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/printing/tfoot-repeats-at-bottom-of-each-page-multiple-tables-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/printing/thead-repeats-at-top-of-each-page-multiple-tables-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win/paint/printing/print-text-shadow-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win/printing/fixed-positioned-headers-and-footers-absolute-covering-some-pages-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win/printing/forced-break-tree-dump-only-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win/printing/iframe-print-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win/printing/list-item-with-empty-first-line-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win/printing/tfoot-repeats-at-bottom-of-each-page-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win/printing/tfoot-repeats-at-bottom-of-each-page-multiple-tables-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win/printing/thead-repeats-at-top-of-each-page-multiple-tables-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win7/printing/absolute-position-headers-and-footers-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win7/printing/ellipsis-printing-style-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win7/printing/fixed-positioned-but-static-headers-and-footers-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win7/printing/fixed-positioned-headers-and-footers-clipped-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win7/printing/fixed-positioned-headers-and-footers-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win7/printing/fixed-positioned-headers-and-footers-inside-transform-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win7/printing/fixed-positioned-headers-and-footers-larger-than-page-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/printing/simultaneous-position-float-change-expected.txt
M third_party/WebKit/Source/core/layout/LayoutTreeAsText.cpp
M third_party/WebKit/Source/core/layout/LayoutView.cpp
M third_party/WebKit/Source/core/page/PrintContext.cpp
M third_party/WebKit/Source/core/paint/FramePainter.cpp
M third_party/WebKit/Source/core/paint/ViewPainter.cpp
53 files changed, 10,097 insertions(+), 80 deletions(-)