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)
Toolkit
Printing
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)
9.76 MB,
video/mp4
|
Details | |
1.33 KB,
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
[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.
Assignee | ||
Comment 2•8 years ago
|
||
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)
Reporter | ||
Comment 3•8 years ago
|
||
(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)
Updated•8 years ago
|
Priority: -- → P3
Assignee | ||
Comment 4•8 years ago
|
||
(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)
Comment 5•8 years ago
|
||
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)
Assignee | ||
Comment 6•8 years ago
|
||
This patch closes a pending progress dialog before we allow another one to be opened.
Attachment #8803104 -
Flags: feedback?(mconley)
Comment 7•8 years ago
|
||
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+
Assignee | ||
Comment 8•8 years ago
|
||
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)
Updated•8 years ago
|
Assignee: nobody → mlongaray
Comment 9•8 years ago
|
||
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+
Comment 10•8 years ago
|
||
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
Comment 11•8 years ago
|
||
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
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f4459d4ba9ba
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Assignee | ||
Comment 13•7 years ago
|
||
Hey Bogdan, could you please test and verify this bug?
Flags: needinfo?(bogdan.maris)
Reporter | ||
Comment 14•7 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•