Skip to content

Commit

Permalink
"Print" duplicate PDF URL rectangles at the correct offset.
Browse files Browse the repository at this point in the history
Because of crbug.com/1392701 we currently output URL rectangles (for
clickable links in PDFs) for *all* links in the container, *for each
fragment* generated by the container, unconditionally.

The way it currently works is to traverse the LayoutObject tree and
calculate rectangles using LayoutObject::OutlineRects(), which returns
rectangles of the link relatively to the first fragment generated by the
container. So, when converting this to absolute offsets, it would only
be correct in the first fragment. To fix this, translate the paint
offset to that of the first fragment when handling subsequent fragments.

In order to add a meaningful test, I had to add support for printing
other pages than just the first one in print_context_test.cc, and also
make sure that the page rectangle used is the one calculated internally
(which is typically 33% larger than specified in each direction), so
that stuff at the bottom of a page doesn't get clipped away.

(cherry picked from commit 3e492d7)

Bug: 1392642
Change-Id: Iaa88aa5b1e438241c48db588b0410503adebb3f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4047783
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Auto-Submit: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1074671}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4061294
Reviewed-by: Krishna Govind <govind@chromium.org>
Commit-Queue: Srinivas Sista <srinivassista@chromium.org>
Owners-Override: Srinivas Sista <srinivassista@chromium.org>
Cr-Commit-Position: refs/branch-heads/5359@{#1015}
Cr-Branched-From: 27d3765-refs/heads/main@{#1058933}
  • Loading branch information
mstensho authored and SumaliSonchai committed Nov 28, 2022
1 parent b3d801b commit a59d9cb
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 4 deletions.
61 changes: 58 additions & 3 deletions third_party/blink/renderer/core/page/print_context_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,17 @@ class PrintContextTest : public PaintTestConfigurations, public RenderingTest {
GetDocument().body()->setInnerHTML(body_content);
}

void PrintSinglePage(SkCanvas& canvas) {
gfx::Rect page_rect(0, 0, kPageWidth, kPageHeight);
gfx::Rect PrintSinglePage(SkCanvas& canvas, int page_number = 0) {
GetDocument().SetPrinting(Document::kBeforePrinting);
Event* event = MakeGarbageCollected<BeforePrintEvent>();
GetPrintContext().GetFrame()->DomWindow()->DispatchEvent(*event);
GetPrintContext().BeginPrintMode(page_rect.width(), page_rect.height());
GetPrintContext().BeginPrintMode(kPageWidth, kPageHeight);
GetDocument().View()->UpdateAllLifecyclePhasesExceptPaint(
DocumentUpdateReason::kTest);

GetPrintContext().ComputePageRects(gfx::SizeF(kPageWidth, kPageHeight));
gfx::Rect page_rect = GetPrintContext().PageRect(page_number);

auto* builder = MakeGarbageCollected<PaintRecordBuilder>();
GraphicsContext& context = builder->Context();
context.SetPrinting(true);
Expand All @@ -151,6 +154,7 @@ class PrintContextTest : public PaintTestConfigurations, public RenderingTest {
}
builder->EndRecording()->Playback(&canvas);
GetPrintContext().EndPrintMode();
return page_rect;
}

static String AbsoluteBlockHtmlForLink(int x,
Expand Down Expand Up @@ -466,6 +470,57 @@ TEST_P(PrintContextTest, LinkTargetBoundingBox) {
EXPECT_SKRECT_EQ(50, 60, 200, 100, operations[0].rect);
}

TEST_P(PrintContextTest, LinkInFragmentedContainer) {
SetBodyInnerHTML(R"HTML(
<style>
body {
margin: 0;
line-height: 50px;
orphans: 1;
widows: 1;
}
</style>
<div style="height:calc(100vh - 90px);"></div>
<div>
<a href="http://www.google.com">link 1</a><br>
<!-- Page break here. -->
<a href="http://www.google.com">link 2</a><br>
<a href="http://www.google.com">link 3</a><br>
</div>
)HTML");

MockPageContextCanvas first_page_canvas;
gfx::Rect page_rect = PrintSinglePage(first_page_canvas, 0);
Vector<MockPageContextCanvas::Operation> operations =
first_page_canvas.RecordedOperations();

// TODO(crbug.com/1392701): Should be 1.
ASSERT_EQ(operations.size(), 3u);

const auto& page1_link1 = operations[0];
EXPECT_EQ(page1_link1.type, MockPageContextCanvas::kDrawRect);
EXPECT_GE(page1_link1.rect.y(), page_rect.height() - 90);
EXPECT_LE(page1_link1.rect.bottom(), page_rect.height() - 40);

MockPageContextCanvas second_page_canvas;
page_rect = PrintSinglePage(second_page_canvas, 1);
operations = second_page_canvas.RecordedOperations();

// TODO(crbug.com/1392701): Should be 2.
ASSERT_EQ(operations.size(), 3u);
// TODO(crbug.com/1392701): Should be operations[0]
const auto& page2_link1 = operations[1];
// TODO(crbug.com/1392701): Should be operations[1]
const auto& page2_link2 = operations[2];

EXPECT_EQ(page2_link1.type, MockPageContextCanvas::kDrawRect);
EXPECT_GE(page2_link1.rect.y(), page_rect.y());
EXPECT_LE(page2_link1.rect.bottom(), page_rect.y() + 50);
EXPECT_EQ(page2_link2.type, MockPageContextCanvas::kDrawRect);
EXPECT_GE(page2_link2.rect.y(), page_rect.y() + 50);
EXPECT_LE(page2_link2.rect.bottom(), page_rect.y() + 100);
}

// Here are a few tests to check that shrink to fit doesn't mess up page count.

TEST_P(PrintContextTest, ScaledVerticalRL1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -821,8 +821,20 @@ void NGBoxFragmentPainter::PaintLineBoxes(const PaintInfo& paint_info,

if (child_paint_info.phase == PaintPhase::kForeground &&
child_paint_info.ShouldAddUrlMetadata()) {
// TODO(crbug.com/1392701): Avoid walking the LayoutObject tree (which is
// what AddURLRectsForInlineChildrenRecursively() does). We should walk the
// fragment tree instead (if we can figure out how to deal with culled
// inlines - or get rid of them). Walking the LayoutObject tree means that
// we'll visit every link in the container for each fragment generated,
// leading to duplicate entries. This is only fine as long as the absolute
// offsets is the same every time a given link is visited. Otherwise links
// might end up as unclickable in the resulting PDF. So make sure that the
// paint offset relative to the first fragment generated by this
// container. This matches legacy engine behavior.
PhysicalOffset paint_offset_for_first_fragment =
paint_offset - OffsetInStitchedFragments(box_fragment_);
AddURLRectsForInlineChildrenRecursively(*layout_object, child_paint_info,
paint_offset);
paint_offset_for_first_fragment);
}

// If we have no lines then we have no work to do.
Expand Down

0 comments on commit a59d9cb

Please sign in to comment.