Closed Bug 1306298 Opened 8 years ago Closed 8 years ago

Print preview window showing progress stuck when double clicking Simplify Page

Categories

(Toolkit :: Printing, defect, P3)

defect

Tracking

()

VERIFIED FIXED
mozilla52
Tracking Status
firefox49 --- unaffected
firefox50 --- affected
firefox51 --- affected
firefox52 --- verified
firefox53 --- verified

People

(Reporter: bmaris, Assigned: mlongaray)

References

Details

Attachments

(2 files, 1 obsolete file)

[Affected versions]:
- Firefox 50 beta 2
- latest Nightly 52.0a1

[Affected platforms]:
- Windows 7 64 bit
- Windows 10 64 bit
- Ubuntu 16.04 32 bit and 64 bit

[Steps to reproduce]:
1. Start Firefox
2. Visit a page with Reader View (ex: wikipedia.org)
3. Go to File -> Print Preview
4. Fast double click "Simplify Page"
5. Repeat step 4 a few times

[Expected result]:
- Page switches from Preview to Simplify with no issues recorder

[Actual result]:
- Print preview progress pop-up is stuck on page and if step 4 is repeated multiple times, multiple pop-ups are displayed

[Regression range]:
- This is not a recent regression, it reproduces on the old Nightly from 2016-06-06 where Simplify page was introduced. 

[Additional notes]:
- Screencast showing the issue attached.
Hey mlongaray, feel like poking at this one?
Flags: needinfo?(mlongaray)
Hey Mike, I think I'd stumbled on this issue in the past, but I thought it was fixed after some tests.

I downloaded release 48 and managed to reproduce the same issue by toggling the layout (portrait/landscape). Print preview progress pop-up also gets stuck when layout gets changed too quickly. I attached a screen recording to confirm this.

It seems like this issue is not tied to simplify page feature itself, since release 48 did not contain any simplify-related code.

It is probably a synchronicity issue. If somehow we could block the toolbar until print progress pop-up is done, I think we would not have this problem. What do you think?

PS: if we close the print progress pop-up after it gets stuck, it will not pop up again, even if we re-open print preview. After I restarted the browser, the pop-up appeared normally on the screen again.

Following is the link and version that I used for testing. Could you please also confirm this, Bogdan?

Version tested: firefox-48.0.en GB.win64 - 26-Jul-2016 02:22 (https://ftp.mozilla.org/pub/firefox/nightly/2016/07/2016-07-25-09-36-59-mozilla-release-l10n/)
Flags: needinfo?(mlongaray) → needinfo?(bogdan.maris)
(In reply to Matheus Longaray from comment #2)
> Created attachment 8796289 [details]
> Print preview progress pop-up stuck on release 48
> 
> Hey Mike, I think I'd stumbled on this issue in the past, but I thought it
> was fixed after some tests.
> 
> I downloaded release 48 and managed to reproduce the same issue by toggling
> the layout (portrait/landscape). Print preview progress pop-up also gets
> stuck when layout gets changed too quickly. I attached a screen recording to
> confirm this.
> 
> It seems like this issue is not tied to simplify page feature itself, since
> release 48 did not contain any simplify-related code.
> 
> It is probably a synchronicity issue. If somehow we could block the toolbar
> until print progress pop-up is done, I think we would not have this problem.
> What do you think?
> 
> PS: if we close the print progress pop-up after it gets stuck, it will not
> pop up again, even if we re-open print preview. After I restarted the
> browser, the pop-up appeared normally on the screen again.
> 
> Following is the link and version that I used for testing. Could you please
> also confirm this, Bogdan?
> 
> Version tested: firefox-48.0.en GB.win64 - 26-Jul-2016 02:22
> (https://ftp.mozilla.org/pub/firefox/nightly/2016/07/2016-07-25-09-36-59-
> mozilla-release-l10n/)

I can confirm the above statements with 48 build switching between portrait/landscape but only on Windows OS. Closing print progress will mean that in the current session it will not show up again, restarting will make it show up again. .
On Ubuntu though, after closing the print progress pop-up, others will open if I keep switch between portrait/landscape.
Flags: needinfo?(bogdan.maris)
Priority: -- → P3
(In reply to Matheus Longaray from comment #2)
> Created attachment 8796289 [details]
> Print preview progress pop-up stuck on release 48
> 
> Hey Mike, I think I'd stumbled on this issue in the past, but I thought it
> was fixed after some tests.
> 
> I downloaded release 48 and managed to reproduce the same issue by toggling
> the layout (portrait/landscape). Print preview progress pop-up also gets
> stuck when layout gets changed too quickly. I attached a screen recording to
> confirm this.
> 
> It seems like this issue is not tied to simplify page feature itself, since
> release 48 did not contain any simplify-related code.
> 
> It is probably a synchronicity issue. If somehow we could block the toolbar
> until print progress pop-up is done, I think we would not have this problem.
> What do you think?
> 

Hey Mike, if we go forward with this approach, do you think bug 1306297 and bug 1306295 would also likely be fixed? What do you suggest?

> PS: if we close the print progress pop-up after it gets stuck, it will not
> pop up again, even if we re-open print preview. After I restarted the
> browser, the pop-up appeared normally on the screen again.
> 
> Following is the link and version that I used for testing. Could you please
> also confirm this, Bogdan?
> 
> Version tested: firefox-48.0.en GB.win64 - 26-Jul-2016 02:22
> (https://ftp.mozilla.org/pub/firefox/nightly/2016/07/2016-07-25-09-36-59-
> mozilla-release-l10n/)
Flags: needinfo?(mconley)
Hm. I'd rather not lock the toolbar if we can avoid it. What might be better is to make sure that if there's a pre-existing dialog, that we close it before we allow another one to be opened.

Looking at http://searchfox.org/mozilla-central/rev/d96317a351af8aa78ab9847e7feed964bbaac7d7/embedding/components/printingui/unixshared/nsPrintingPromptService.cpp#261 it looks like if you send a STATE_STOP to the _webProgressPP in printUtils.js, that should be enough to close the window. Perhaps it'd be enough to send that in the event that the user clicks on anything that might bring up the progress bar (or if they exit print preview).
Flags: needinfo?(mconley)
This patch closes a pending progress dialog before we allow another one to be opened.
Attachment #8803104 - Flags: feedback?(mconley)
Comment on attachment 8803104 [details] [diff] [review]
WIP1: Get rid of a pre-existing progress dialog

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

Thanks!

::: toolkit/components/printing/content/printUtils.js
@@ +209,5 @@
>        ppBrowser.collapsed = true;
> +
> +      // If the user transits too quickly within preview and we have a pending
> +      // progress dialog, we will close it before opening a new one.
> +      if (this._webProgressPP && this._webProgressPP.value)

Let's brace this, even though it's a one-liner. Unbraced one-liners give me the willies.
Attachment #8803104 - Flags: feedback?(mconley) → feedback+
This patch closes pending progress dialog before we allow another one to be opened.
Attachment #8803104 - Attachment is obsolete: true
Attachment #8805155 - Flags: review?(mconley)
Assignee: nobody → mlongaray
Comment on attachment 8805155 [details] [diff] [review]
WIP2: Add missing braces

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

Looks good to me! Thanks.
Attachment #8805155 - Flags: review?(mconley) → review+
Author: Matheus Longaray <mlongaray@hp.com>
Bug number: 1306298
Commit message: "Bug 1306298 - Close any pre-existing print progress dialogs when on re-entry to print preview. r=mconley"
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4459d4ba9ba
Close any pre-existing print progress dialogs when on re-entry to print preview. r=mconley
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f4459d4ba9ba
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Hey Bogdan, could you please test and verify this bug?
Flags: needinfo?(bogdan.maris)
(In reply to Matheus Longaray (:mlongaray) from comment #13)
> Hey Bogdan, could you please test and verify this bug?

Sure thing!

I did verify this on Windows 10 64-bit, Windows 7-64bit and Ubuntu 16.04 32-bit using latest Nightly 53.0a1 and latest Developer Edition 52.0a2. No window is left open when switching through landscape/portrait or activate/deactivate simplify page mode.

Changing the status of the bug to Verified Fixed but leaving the tracking flag for 51 as it is since we want to verify it there as well when it reaches beta.
Status: RESOLVED → VERIFIED
Flags: needinfo?(bogdan.maris)
Blocks: 962433
You need to log in before you can comment on or make changes to this bug.