Issue metadata
Sign in to add a comment
|
Does not print correctly, missing parts of page.
Reported by
thoux2...@gmail.com,
Sep 13
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.92 Safari/537.36 Steps to reproduce the problem: 1. Log into identifix.com and click on Free Test Drive 2. click on View traditional Maintenance Schedules 3. click print What is the expected behavior? Print the document What went wrong? Check marks within the document do not appear in print Did this work before? Yes 68 Chrome version: 69.0.3497.92 Channel: stable OS Version: 10.0 Flash Version:
,
Sep 14
,
Sep 14
Do you have a set of test credentials for this page, or a page that does not require a login that has the same problem, so that we can try to reproduce the issue?
,
Sep 14
I've attached a video to reproduce the issue without a log on. https://www.identifix.com/
,
Sep 14
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 14
This bisects to https://chromium.googlesource.com/chromium/src/+log/cef2ea9d623b14a4d7e270e76419c222e96478b3..66da0a7ac0283b79781efed0f59cd9305a939230 Two of those changes are Mac and iOS only, so it has to be one of the two blink changes. Guessing https://chromium-review.googlesource.com/c/chromium/src/+/1095894? cc-ing kojii@.
,
Sep 14
,
Sep 14
Unable to reproduce the issue on reported chrome version #69.0.3497.92 and latest canary 71.0.3552.2 using windows 10 by following below steps. Steps: ===== 1.Launched chrome. 2.Navigated to "identifix.com". 3.Clicked on "A free test drive". 4.When a page opened clicked on "Test Drive". 5.After it navigates to a page under maintenance tab clicked on "maintenance schedule". 6.Opened the print preview of the page. 7.Under more settings options, checked "background graphics". 8.Observed check marks within the document appear in print preview. Attached screencast for reference. @reporter: Could you please review the attached screencast and let us know if anything is being missed her and requesting you to confirm whether the issue is seen on printed page or print preview. Thanks.!
,
Sep 14
Comment 8: You need to click the print button in the webpage (far right side), rather than directly opening print preview.
,
Sep 26
Clicking Background Graphics does not change the check marks being displayed.
,
Sep 26
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 26
Confirmed the bisect in comment #6. Assigning.
,
Oct 9
I can't imagine how the CL in #6 affects print preview, but I also don't know how print preview kicks layout. I'll try to revert it and see if it fixes, I hope it's not too hard.
,
Oct 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/209909630cf94d4568bc62157053bae5791908eb commit 209909630cf94d4568bc62157053bae5791908eb Author: Koji Ishii <kojii@chromium.org> Date: Tue Oct 09 11:01:14 2018 Revert "Improve performance of PreviousBreakOpportunity" This reverts commit 99d504fd35a7e07c4cf16bbbe1464044e2195b4e. Reason for revert: crbug.com/883963 bisected to this CL. I'm not sure how this affects print preview, reverting to see the effect. This revert may affect LayoutNG performance, but should not affect the currently shipping line breaker. Original change's description: > Improve performance of PreviousBreakOpportunity > > In the current layout engine, PreviousBreakOpportunity is used > only when mid-word break (break-all or break-word), but it is > much more heavily used in LayoutNG. > > LazyLineBreakIterator is designed for forward only. > PreviousBreakOpportunity is implemented by repeatedly calling > NextBreakablePosition, but since NextBreakablePosition look > for the next break opportunity until the end of the string, > when a very long word without break opportunity is given, > PreviousBreakOpportunity is O(n!). > > This patch changes it to O(n) by limiting the end position > NextBreakablePosition can look for. > > blink/perf_tests/layout/word-break-break-word.html consumes > 78% of the total time in LayoutNG. The average run is: > Current engine: 469ms > LayoutNG: 26,644ms > This patch: 2,250ms > > It's still 4-5 times slower, more improvements will be in > following patches. > > Bug: 636993 > Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng > Change-Id: I814e2c45c8030aa682c7f5e3a3b785b3c0733c84 > Reviewed-on: https://chromium-review.googlesource.com/1095894 > Commit-Queue: Koji Ishii <kojii@chromium.org> > Reviewed-by: Emil A Eklund <eae@chromium.org> > Cr-Commit-Position: refs/heads/master@{#567133} TBR=eae@chromium.org,kojii@chromium.org Bug: 636993, 883963 Change-Id: I2c453a011208b014e342d7b44646eb90b5b7bbd2 Reviewed-on: https://chromium-review.googlesource.com/c/1270396 Commit-Queue: Koji Ishii <kojii@chromium.org> Reviewed-by: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/master@{#597876} [modify] https://crrev.com/209909630cf94d4568bc62157053bae5791908eb/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/fast/text/midword-break-before-surrogate-pair-expected.png [modify] https://crrev.com/209909630cf94d4568bc62157053bae5791908eb/third_party/blink/renderer/platform/text/text_break_iterator.cc [modify] https://crrev.com/209909630cf94d4568bc62157053bae5791908eb/third_party/blink/renderer/platform/text/text_break_iterator.h
,
Oct 11
The revert in #14 landed in according to: 71.0.3575.0 https://storage.googleapis.com/chromium-find-releases-static/index.html Now as I test 71.0.3576.0, this still reproduces. Can you please run bisect again?
,
Oct 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/786b1994bf6f43f2ff038894a1891435f2de22b9 commit 786b1994bf6f43f2ff038894a1891435f2de22b9 Author: Koji Ishii <kojii@chromium.org> Date: Thu Oct 11 14:22:14 2018 Reland "Improve performance of PreviousBreakOpportunity" This reverts commit 209909630cf94d4568bc62157053bae5791908eb. Reason for revert: The revert did not fix crbug.com/883963 . Requested to re-run bisect. Original change's description: > Revert "Improve performance of PreviousBreakOpportunity" > > This reverts commit 99d504fd35a7e07c4cf16bbbe1464044e2195b4e. > > Reason for revert: > > crbug.com/883963 bisected to this CL. I'm not sure how this > affects print preview, reverting to see the effect. > > This revert may affect LayoutNG performance, but should not > affect the currently shipping line breaker. > > Original change's description: > > Improve performance of PreviousBreakOpportunity > > > > In the current layout engine, PreviousBreakOpportunity is used > > only when mid-word break (break-all or break-word), but it is > > much more heavily used in LayoutNG. > > > > LazyLineBreakIterator is designed for forward only. > > PreviousBreakOpportunity is implemented by repeatedly calling > > NextBreakablePosition, but since NextBreakablePosition look > > for the next break opportunity until the end of the string, > > when a very long word without break opportunity is given, > > PreviousBreakOpportunity is O(n!). > > > > This patch changes it to O(n) by limiting the end position > > NextBreakablePosition can look for. > > > > blink/perf_tests/layout/word-break-break-word.html consumes > > 78% of the total time in LayoutNG. The average run is: > > Current engine: 469ms > > LayoutNG: 26,644ms > > This patch: 2,250ms > > > > It's still 4-5 times slower, more improvements will be in > > following patches. > > > > Bug: 636993 > > Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng > > Change-Id: I814e2c45c8030aa682c7f5e3a3b785b3c0733c84 > > Reviewed-on: https://chromium-review.googlesource.com/1095894 > > Commit-Queue: Koji Ishii <kojii@chromium.org> > > Reviewed-by: Emil A Eklund <eae@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#567133} > > TBR=eae@chromium.org,kojii@chromium.org > > Bug: 636993, 883963 > Change-Id: I2c453a011208b014e342d7b44646eb90b5b7bbd2 > Reviewed-on: https://chromium-review.googlesource.com/c/1270396 > Commit-Queue: Koji Ishii <kojii@chromium.org> > Reviewed-by: Koji Ishii <kojii@chromium.org> > Cr-Commit-Position: refs/heads/master@{#597876} TBR=eae@chromium.org,kojii@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 636993, 883963 Change-Id: I8e9881deca736c5b574dc287d04f9c2fffe76680 Reviewed-on: https://chromium-review.googlesource.com/c/1275985 Reviewed-by: Koji Ishii <kojii@chromium.org> Commit-Queue: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/master@{#598754} [modify] https://crrev.com/786b1994bf6f43f2ff038894a1891435f2de22b9/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/fast/text/midword-break-before-surrogate-pair-expected.png [modify] https://crrev.com/786b1994bf6f43f2ff038894a1891435f2de22b9/third_party/blink/renderer/platform/text/text_break_iterator.cc [modify] https://crrev.com/786b1994bf6f43f2ff038894a1891435f2de22b9/third_party/blink/renderer/platform/text/text_break_iterator.h
,
Oct 12
Revering the change didn't help, would you mind re-running the bisect schenney or are we better of having the printing team triage this?
,
Oct 14
I'll look but probably not until late Monday or Tuesday.
,
Oct 15
Able to reproduce the issue on chrome version #71.0.3576.0 as per comment #15 on Windows 10 and Ubuntu 17.10 by following steps as per comment #9.The following is the bisect information. Note: Could not check the issue on Mac Os as the canary in Mac is still in M-71. Bisect information: ============== Good build - 71.0.3578.0 Bad Build - 72.0.3579.0 Note: Unable to provide Change Log from per-revision-bisect and chromium bisect as it was giving an error as "RuntimeError: We don't have enough builds to bisect." Thanks.!
,
Oct 15
Can you please bisect between 68 and 69.0.3497.92, as in #0? Also, this seems sometimes flaky. For one I noticed, it does not reproduce once I open Inspector, but there maybe more.
,
Oct 16
@kojii: As per comment#20 tried bisecting the issue but found inconsistent behavior, hence unable to provide bisect information from our end. Requesting schenney@ to look into the issue. Thanks.!
,
Oct 30
,
Oct 30
I get the same bisect range as comment #6, which again suggests kojii@'s patch. I'll try to bisect around the revert to see if that did or did not fix it.
,
Oct 30
The revert apparently did not fix it, so maybe we're blaming the wrong thing. I redid the original bisect and get the same range. Now to try to verify on a ToT build and locally revert the other patches in the revision range.
,
Oct 30
As commented in #20, this sometimes does not repro, like after I opened Inspector. If r562150 is possible, maybe CSSInBodyDoesNotBlockPaint causes paint before we evaluate CSS?
,
Oct 30
,
Oct 30
pmeenan@ might know better, but from the name "CSSInBodyDoesNotBlockPaint", I'm guessing we might be taking the PDF before we evaluate CSS in body because paint occurred? That could explain the flakiness if so.
,
Dec 14
Any status update on this bug?
,
Dec 15
I shouldn't own this. Over to pmeenan@ to see if disabling r562150 solves the problem.
,
Dec 27
,
Dec 27
This issue was indeed caused by the CSSInBodyDoesNotBlockPaint feature. What happens is that when printing, the app dynamically creates an iframe with the printed contents, presumably in response to the beforeprint event or when clicking the print button. This iframe has multiple external stylesheets, but the one that contains the checkbox image CSS is the same stylesheet as is present in the main frame. Before CSSInBodyDoesNotBlockPaint was turned on, the parser would not stop parsing while waiting for that style sheet. By the end of parsing, the style sheet had loaded because it was already in the memory cache (due to it being in the style sheet list for the main frame). Now it does, which appears to cause a brief yield of the document parser, and then a render, without the style sheet. The site should not be relying on this behavior, and instead set up the iframe in advance and use media queries to change style. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by krajshree@chromium.org
, Sep 14