Closed Bug 1285544 Opened 8 years ago Closed 8 years ago

Pages printed with Simplify Page show "about:blank" in the upper right corner instead of the page URL

Categories

(Toolkit :: Printing, defect, P3)

Product:
Toolkit
Component:
Printing
Platform:
x86_64
All
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla50
Tracking Flags:
Tracking Status
firefox50 --- verified

People

(Reporter: cmuresan, Assigned: mlongaray)

References

Details

Attachments

(2 files, 1 obsolete file)

Pages printed with Simplify Page show aboutblank.mp4
6.75 MB, video/mp4
Details
WIP2: Show proper URL when printing with Simplify Page
2.32 KB, patch
mconley
: review+
Details | Diff | Splinter Review 
[Affected versions]:
- Nightly 50.0a1

[Affected Platforms]:
- Windows 7 x64, 8.1 x64, 10 x64
- Ubuntu 14.04 x64, 16.04 x64

[Prerequisites]:
- Set the "print.use_simplify_page" pref to true

[Steps to reproduce]:
1. Visit any page that can be simplify-able (eg. https://en.wikipedia.org/wiki/Iraq_Inquiry) 
2. Print Preview, click the "Simplify Page" checkbox and observe the URL in the upper right corner.
3. Print the page to PDF.
4. Open the PDF and observe the URL in the upper right corner of the page.

[Expected result]:
The URL in the upper right corner should be the same as the one seen while print previewing.

[Actual result]:
"about:blank" is printed in the upper right corner instead.

[Notes]:
- Attached a screen recording of the issue.
Priority: -- → P3
Assignee: nobody → mlongaray
This patch addresses the URL problem when printing while using simplify checkbox on print preview.
Attachment #8769324 - Flags: review?(mconley)
Comment on attachment 8769324 [details] [diff] [review]
WIP1: Show proper URL when printing with Simplify Page

Review of attachment 8769324 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch, mlongaray! I think the overall idea is fine, but that the approach could be simplified (heh) a little. See below.

::: toolkit/components/printing/content/printPreviewBindings.xml
@@ +219,5 @@
>  
>        <method name="print">
>          <body>
>          <![CDATA[
> +          PrintUtils.printWindow(this.mPPBrowser.outerWindowID, this.mPPBrowser, this.mSimplifyPageCheckbox.checked);

PrintUtils already knows whether or not the checkbox is checked via _shouldSimplify. I think we can probably get away with not having this state being passed around by just having printWindow send down the _shouldSimplify state variable in the Printing:Print message.

::: toolkit/content/browser-content.js
@@ +637,5 @@
> +
> +    // If we happen to be on simplified mode, we need to set docURL in order
> +    // to generate header/footer content correctly, since simplified tab has
> +    // "about:blank" as its URI.
> +    if (printSettings && simplifiedMode)

Braces around this block, please.
Attachment #8769324 - Flags: review?(mconley) → feedback+
(In reply to Mike Conley (:mconley) - (Needinfo me!) from comment #2)
> Comment on attachment 8769324 [details] [diff] [review]
> WIP1: Show proper URL when printing with Simplify Page
> 
> Review of attachment 8769324 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for the patch, mlongaray! I think the overall idea is fine, but that
> the approach could be simplified (heh) a little. See below.
> 
> ::: toolkit/components/printing/content/printPreviewBindings.xml
> @@ +219,5 @@
> >  
> >        <method name="print">
> >          <body>
> >          <![CDATA[
> > +          PrintUtils.printWindow(this.mPPBrowser.outerWindowID, this.mPPBrowser, this.mSimplifyPageCheckbox.checked);
> 
> PrintUtils already knows whether or not the checkbox is checked via
> _shouldSimplify. I think we can probably get away with not having this state
> being passed around by just having printWindow send down the _shouldSimplify
> state variable in the Printing:Print message.
> 

Oh.. right! Good catch, Mike!

> ::: toolkit/content/browser-content.js
> @@ +637,5 @@
> > +
> > +    // If we happen to be on simplified mode, we need to set docURL in order
> > +    // to generate header/footer content correctly, since simplified tab has
> > +    // "about:blank" as its URI.
> > +    if (printSettings && simplifiedMode)
> 
> Braces around this block, please.
This patch addresses Mike's last feedback
Attachment #8769324 - Attachment is obsolete: true
Attachment #8769732 - Flags: review?(mconley)
Comment on attachment 8769732 [details] [diff] [review]
WIP2: Show proper URL when printing with Simplify Page

Review of attachment 8769732 [details] [diff] [review]:
-----------------------------------------------------------------

This seems to do the job for me. Thanks mlongaray!
Attachment #8769732 - Flags: review?(mconley) → review+
checkin-needed for attachment 8769732 [details] [diff] [review].

Author: Matheus Longaray <mlongaray@hp.com>
Bug number: Bug 1285544
Commit message: Bug 1285544 - Make sure print jobs that go through simplification get the right document URL set. r=mconley.
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/50bfdf605c55
Make sure print jobs that go through simplification get the right document URL set. r=mconley.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/50bfdf605c55
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Flags: qe-verify+
I managed to reproduce the issue on the affected build Nightly 50.0a1 (2016-07-08) on Windows 10 Pro x64

The fix is now verified on Beta 50.0b3 Build 1, using Windows 10 Pro x64 and Ubuntu 16.04 LTS x64
Thank you, Carmen!
Status: RESOLVED → VERIFIED
status-firefox50: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: