Closed
Bug 1308461
Opened 8 years ago
Closed 8 years ago
Switching between Portrait and Landscape in Print Preview replaces PDFs by a blank page
Categories
(Core :: Print Preview, defect, P1)
Tracking
()
People
(Reporter: epinal99-bugzilla2, Assigned: dalmirsilva)
References
Details
(Keywords: regression, Whiteboard: [mozfr-community])
Attachments
(1 file, 2 obsolete files)
3.13 KB,
patch
|
dholbert
:
review+
jwatt
:
review+
jcristau
:
approval-mozilla-aurora+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
STR:
1) open http://basketligakobiet.pl/internalfiles/fckfiles/file/Komunikat%20WR%201_2016_2017_terminarz%20BLK.pdf
2) Go to Print Preview and switch between Portrait and Landscape.
Result: After the 1st switch, the PDF is replaced by a blank page in portrait or landscape.
Regression range:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=1a72dc1810bc0170754b9fbf14a79e7f855346df&tochange=dc5692c66a028e98980527c9a637932b992874b2
Matheus Longaray — Bug 962433 - "Use Reader Mode for printing articles" [r=mconley]
Blocks: 962433
status-firefox49:
--- → affected
status-firefox50:
--- → affected
status-firefox51:
--- → affected
status-firefox52:
--- → affected
tracking-firefox50:
--- → ?
tracking-firefox51:
--- → ?
tracking-firefox52:
--- → ?
Flags: needinfo?(mconley)
Keywords: regression
Updated•8 years ago
|
Assignee: nobody → mlongaray
Flags: needinfo?(mconley)
Updated•8 years ago
|
Priority: -- → P1
Comment 1•8 years ago
|
||
Track 51+ as regression related to print preview between portrait & landscape.
Comment 2•8 years ago
|
||
Too late for 49, marking fix-optional for 50.
Comment 4•8 years ago
|
||
Thanks for finding this, looking into how to fix.
Tracked since it's a new regression since Fx49.
Comment 6•8 years ago
|
||
Status:
Reverted local tree to FIREFOX_AURORA_48_BASE revision (rev 1c6385ae1fe7) in which does not contain any code from bug 962433 (rev dc5692c66a02).
Applied patch from bug 1308621 (rev b5d1247e02ed) that fixes a property clobbering issue we had on printing code.
Weirdly enough, after we have entered on preview, if we change any settings on preview (portrait to landscape per say), preview will generate a blank page. And that will continue to happen as long as we change any setting.
Preview only generates the expected content/output in the first transition.
This issue only happens when previewing PDFs. For normal web pages, preview is still working as expected.
Do you guys have any insights about this one? I'll be glad to provide more info if I can. Thanks.
Flags: needinfo?(mconley)
Flags: needinfo?(gijskruitbosch+bugs)
Comment 7•8 years ago
|
||
(In reply to Matheus Longaray from comment #6)
> Status:
>
> Reverted local tree to FIREFOX_AURORA_48_BASE revision (rev 1c6385ae1fe7) in
> which does not contain any code from bug 962433 (rev dc5692c66a02).
>
> Applied patch from bug 1308621 (rev b5d1247e02ed) that fixes a property
> clobbering issue we had on printing code.
>
> Weirdly enough, after we have entered on preview, if we change any settings
> on preview (portrait to landscape per say), preview will generate a blank
> page. And that will continue to happen as long as we change any setting.
>
> Preview only generates the expected content/output in the first transition.
>
> This issue only happens when previewing PDFs. For normal web pages, preview
> is still working as expected.
>
> Do you guys have any insights about this one? I'll be glad to provide more
> info if I can. Thanks.
What happens without the patch from bug 1308621 ?
And in particular, is the regression window just wrong? If the same issue was present before bug 962433 this isn't a regression... but Loic has been around a long time and so I would be surprised if the window was completely wrong...
Flags: needinfo?(mlongaray)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(epinal99-bugzilla2)
Comment 8•8 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #7)
> (In reply to Matheus Longaray from comment #6)
> > Status:
> >
> > Reverted local tree to FIREFOX_AURORA_48_BASE revision (rev 1c6385ae1fe7) in
> > which does not contain any code from bug 962433 (rev dc5692c66a02).
> >
> > Applied patch from bug 1308621 (rev b5d1247e02ed) that fixes a property
> > clobbering issue we had on printing code.
> >
> > Weirdly enough, after we have entered on preview, if we change any settings
> > on preview (portrait to landscape per say), preview will generate a blank
> > page. And that will continue to happen as long as we change any setting.
> >
> > Preview only generates the expected content/output in the first transition.
> >
> > This issue only happens when previewing PDFs. For normal web pages, preview
> > is still working as expected.
> >
> > Do you guys have any insights about this one? I'll be glad to provide more
> > info if I can. Thanks.
>
> What happens without the patch from bug 1308621 ?
On release 48 without the patch from bug 1308621, preview works as expected when printing PDFs. What I was trying to confirm was that this bug is related to _sourceBrowser property. For some reason, when printing from PDFs, the second transition on preview produces a blank page (and so on).
> And in particular, is the regression window just wrong? If the same issue
> was present before bug 962433 this isn't a regression... but Loic has been
> around a long time and so I would be surprised if the window was completely
> wrong...
This issue was not present before bug 962433, so I think the regression window is correct.
Flags: needinfo?(mlongaray)
Updated•8 years ago
|
Flags: needinfo?(gijskruitbosch+bugs)
I tested again, same regression range.
Flags: needinfo?(epinal99-bugzilla2)
Comment 10•8 years ago
|
||
Huh. Okay.
So here's what's run when we switch the page orientation:
http://searchfox.org/mozilla-central/rev/703b663355467293fad148ab7c2c5ee2b878e4d9/toolkit/components/printing/content/printPreviewBindings.xml#323
We write the orientation to the page settings, save those (which goes into prefs), and then run printPreview again.
Are we perhaps not handling a re-entry into printPreview when we're already in printPreview? Can you investigate that, mlongaray?
Flags: needinfo?(mconley) → needinfo?(mlongaray)
Comment 11•8 years ago
|
||
(In reply to Mike Conley (:mconley) - (high latency on reviews and needinfos) from comment #10)
> Huh. Okay.
>
> So here's what's run when we switch the page orientation:
>
> http://searchfox.org/mozilla-central/rev/
> 703b663355467293fad148ab7c2c5ee2b878e4d9/toolkit/components/printing/content/
> printPreviewBindings.xml#323
>
> We write the orientation to the page settings, save those (which goes into
> prefs), and then run printPreview again.
>
> Are we perhaps not handling a re-entry into printPreview when we're already
> in printPreview? Can you investigate that, mlongaray?
I've debugged it and I can confirm that we are indeed handling a re-entry into printPreview as expected.
I wonder why this bug only happens when printing PDFs. Could be the case that we are not handling a back-end exception? Or something related to our browser hosting the PDF viewer?
Flags: needinfo?(mlongaray) → needinfo?(mconley)
Comment 12•8 years ago
|
||
(In reply to Matheus Longaray from comment #11)
>
> I've debugged it and I can confirm that we are indeed handling a re-entry
> into printPreview as expected.
>
> I wonder why this bug only happens when printing PDFs. Could be the case
> that we are not handling a back-end exception? Or something related to our
> browser hosting the PDF viewer?
I would expect / hope that such an exception would be logged to the Browser Console. Do you see such an exception?
If you use the Browser Content Toolbox to debug browser-content.js, do you see anything strange going on when flipping the orientation?
Flags: needinfo?(mconley) → needinfo?(mlongaray)
Comment 13•8 years ago
|
||
(In reply to Mike Conley (:mconley) - (digging out of review / needinfo backlog) from comment #12)
>
> I would expect / hope that such an exception would be logged to the Browser
> Console. Do you see such an exception?
>
> If you use the Browser Content Toolbox to debug browser-content.js, do you
> see anything strange going on when flipping the orientation?
I've debugged browser-content.js using Browser Content Toolbox and everything is normal, apparently.
When we flip the orientation, we do receive the message to enter on preview passing up the correct window ID (http://searchfox.org/mozilla-central/source/toolkit/content/browser-content.js#445).
And we also do receive the message from the print engine telling us that we've finished re-flowing the document in order to flip our print preview UI, with no errors on that data (http://searchfox.org/mozilla-central/source/toolkit/content/browser-content.js#597).
I'm thinking if it's a problem when we are re-flowing the document. Let me describe a scenario so you can share some possibilities that we can tackle.
1) Open the PDF and enter on preview -> we'll see the PDF on preview as expected
2) Flip the orientation -> we'll see a blank PDF
3) Close preview
4) Enter on preview again -> we'll see the PDF you expected to see on step 2 (settings are being properly saved)
5) Flip the orientation -> we'll see a blank PDF (we'd expect to see the same on step 1)
PS: this issue also happens when we change the scale on preview.
Flags: needinfo?(mlongaray) → needinfo?(mconley)
Comment 14•8 years ago
|
||
I've been trying to look at this... one thing that's wrong is that we never remove the onEntered listener because the message manager topics that the addMessageListener and removeMessageListener calls use are different (printpreview vs preview). Just fixing that doesn't seem to be enough though.
Comment 16•8 years ago
|
||
Yury, considering this is pdf-js specific, can you help investigate what's going on here?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(ydelendik)
Comment 17•8 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #16)
> Yury, considering this is pdf-js specific, can you help investigate what's
> going on here?
PDF.js is using onbeforeprint/onafterprint events and mozPrintCallback. Looks like the latter is working. However the events looks weird before and after the patch. Before the regression (FF45ESR) there was:
onbeforeprint event
mozPrintCallback calls
<page orientation changes>
onafterprint event
<page orientation changes>
mozPrintCallback calls
<page orientation changes>
mozPrintCallback calls
...
After regression (Nightly):
onbeforeprint event
mozPrintCallback calls
<page orientation changes>
onbeforeprint event
onafterprint event
<page orientation changes>
onbeforeprint event
onafterprint event
...
Just to notice onafterprint event destroys temporary canvases (w/mozPrintCallback) that are created at onafterprint. So before regression onafterprint was called, but mozPrintCallbacks were called; and after regression onafterprint was called out of sequence and mozPrintCallbacks were not called.
Flags: needinfo?(ydelendik)
Comment 18•8 years ago
|
||
I'm afraid I don't have the cycles to investigate this in the short term. mlongaray, do you?
Flags: needinfo?(mconley) → needinfo?(mlongaray)
Comment 19•8 years ago
|
||
(I feel like bisecting the patch that originally introduced this regression would be a good start)
Comment 20•8 years ago
|
||
(In reply to Mike Conley (:mconley) from comment #18)
> I'm afraid I don't have the cycles to investigate this in the short term.
> mlongaray, do you?
Yes, Mike. I started investigating this one but I had to put it aside for a bit due bug 1306294 (priority). I'll probably get back to it as soon as we are done with the aforementioned bug. I'll let you know otherwise.
Flags: needinfo?(mlongaray)
Assignee | ||
Comment 21•8 years ago
|
||
Hi, guys!
Today I was investigating this issue and I think I found the root cause of the problem.
First, why did it pass before our patch? Because we were using the wrong browser as the source for
print preview (the print preview browser itself); which means we were not previewing the source browser as we have to.
See this snippet from rpintUtils.js:
```js
// collapse the browser here -- it will be shown in
// enterPrintPreview; this forces a reflow which fixes display
// issues in bug 267422.
this._sourceBrowser = this._listener.getPrintPreviewBrowser();
this._sourceBrowser.collapsed = true;
```
correctly became:
```js
let ppBrowser = this._listener.getPrintPreviewBrowser();
ppBrowser.collapsed = true;
```
With this fix, we now preview the source browser, because we are not overwriting it anymore.
Now the PDF viewer has to render the canvas correctly, not only the first time we open the PP,
but every time we interact with the PP settings. That means we have to send the messages
to pdfjs/content/web/viewer.js in the correct order.
As pointer by Yury Delendik, these are the messages we are sending:
```
onbeforeprint event
mozPrintCallback calls
<page orientation changes>
onbeforeprint event
onafterprint event
<page orientation changes>
onbeforeprint event
onafterprint event
```
We are immediately sending an "onafterprint" event, right after the "onbeforeprint", destroying the canvas.
Why?
In nsDocumentViewer.cpp, nsDocumentViewer::PrintPreview is called from browser_content.js#enterPrintPreview.
This method is called every time we change some setting. When it is called it instantiate this:
```c++
nsAutoPtr<AutoPrintEventDispatcher> autoBeforeAndAfterPrint(
new AutoPrintEventDispatcher(doc));
```
It sends the first "onbeforeprint event". A few lines down it checks if there is some print callback canvas,
which is true (that's how PDF viewer draws the PDF).
```c++
if (mPrintEngine->HasPrintCallbackCanvas()) {
mAutoBeforeAndAfterPrint = autoBeforeAndAfterPrint;
}
```
It stores this reference in the mAutoBeforeAndAfterPrint attribute (which was nil), postponing autoBeforeAndAfterPrint destruction (which sends the "onbeforeprint" event).
Until now we are OK! But what happens if we change the orientation? This method gets called again! A new "onbeforeprint" event is sent when
it instantiates another instance of AutoPrintEventDispatcher. After, the 'if' is evaluated again, and the block is executed, but we already have an instance stored in mAutoBeforeAndAfterPrint, then, we overwrite this attribute, causing the previous instance to be destroyed, sending the "onafterprint" event. Bug! It will destroy the canvas, not given the chance to the mozPrintCallback
be executed.
The solution:
Before creating a new AutoPrintEventDispatcher instance, lets make sure the previous is destroyed:
```c++
// Next line
mAutoBeforeAndAfterPrint = nullptr;
nsAutoPtr<AutoPrintEventDispatcher> autoBeforeAndAfterPrint(
new AutoPrintEventDispatcher(doc));
```
It appears to solve the problem!
Let me know if it makes sense!
Comment 22•8 years ago
|
||
Mike, can you check the proposed solution in comment 21 ?
Flags: needinfo?(mconley)
Comment 23•8 years ago
|
||
(In reply to dalmirdasilva from comment #21)
Holy smokes, what an analysis! Great job!
Your solution sounds reasonable, but I'd like to run it by someone else who's been in this lower-level of our printing engine more recently.
jwatt - any problems with the above solution?
Flags: needinfo?(mconley) → needinfo?(jwatt)
Comment 24•8 years ago
|
||
Can you guys please assign Dalmir to the bug instead?
We've been investigating this one together and I think he has nailed it finding the proposed solution.
Assignee | ||
Comment 26•8 years ago
|
||
This patch makes sure previous AutoPrintEventDispatcher instance is destroyed before creating a new one. It prevents us to dispatch events in the wrong order.
Attachment #8817193 -
Flags: feedback?(mconley)
Attachment #8817193 -
Flags: feedback?(jwatt)
Comment 27•8 years ago
|
||
Comment on attachment 8817193 [details] [diff] [review]
WIP1: fix dispatch order of print events
Review of attachment 8817193 [details] [diff] [review]:
-----------------------------------------------------------------
This makes sense to me, assuming it makes sense to fire those callbacks when we're in the process of modifying our print preview state. Let's see what jwatt thinks.
dholbert, who reviewed the patches in bug 1313386, might also be a good source of feedback if jwatt is bogged down.
Attachment #8817193 -
Flags: feedback?(mconley) → feedback+
Assignee | ||
Updated•8 years ago
|
Attachment #8817193 -
Flags: feedback?(dholbert)
Comment 28•8 years ago
|
||
This slipped 50 already and it's getting close to the point of missing 51 as well. Hopefully we can get this patch reviewed and landed soon?
status-firefox53:
--- → affected
tracking-firefox53:
--- → ?
Comment 29•8 years ago
|
||
Sorry for the delay here!
> dholbert, who reviewed the patches in bug 1313386, might also be a good source of feedback if jwatt is bogged down.
As I recall, my review on that bug was mostly sanity-checking that the patches preserve current behavior. I don't really have a good grasp of what events we expect to fire/coalesce/cancel during printing. So, I think it'd really be best for jwatt to comment on this, if he could, since he's taken the most recent comprehensive look at these events & when they need to be fired (in bug 1313386).
I agree with mconley that the fix *seems* sane, though!
Comment 30•8 years ago
|
||
Comment on attachment 8817193 [details] [diff] [review]
WIP1: fix dispatch order of print events
Review of attachment 8817193 [details] [diff] [review]:
-----------------------------------------------------------------
::: layout/base/nsDocumentViewer.cpp
@@ +3890,5 @@
> nsCOMPtr<nsIDocument> doc = window->GetDoc();
> NS_ENSURE_STATE(doc);
>
> + // If we have a pending 'afterprint', this makes sure we dispatch it
> + // prior to allocate new AutoPrintEventDispatcher.
Two extreme nits:
(1) There's a trailing space character at the end of the first added line here.
(2) "prior to allocate" seems grammatically awkward. Perhaps "prior to allocating" would make more sense.
Assignee | ||
Comment 31•8 years ago
|
||
Comment on attachment 8817193 [details] [diff] [review]
WIP1: fix dispatch order of print events
Hi, Olli
Could you also take a look at this patch?
See more at: https://bugzilla.mozilla.org/show_bug.cgi?id=1308461#c21
Attachment #8817193 -
Flags: feedback?(dholbert) → feedback?(bugs)
Comment 32•8 years ago
|
||
(In reply to dalmirdasilva from comment #21)
> Hi, guys!
>
> Today I was investigating this issue and I think I found the root cause of
> the problem.
> First, why did it pass before our patch? Because we were using the wrong
> browser as the source for
> print preview (the print preview browser itself);
That was by design (unless I'm misunderstanding the comment here). The idea that once you've print-previewed once, the
original document may do whatever and that doesn't affect to the print preview anymore, since pp is a clone.
Comment 33•8 years ago
|
||
Comment on attachment 8817193 [details] [diff] [review]
WIP1: fix dispatch order of print events
But if this helps, fine. A bit weird to get called nestedly, but this affects to Gecko specific canvas printing only.
Attachment #8817193 -
Flags: feedback?(bugs) → feedback+
Updated•8 years ago
|
Assignee | ||
Comment 34•8 years ago
|
||
Nits addressed.
Attachment #8817193 -
Attachment is obsolete: true
Attachment #8817193 -
Flags: feedback?(jwatt)
Attachment #8824420 -
Flags: review?(mconley)
Attachment #8824420 -
Flags: review?(jwatt)
Comment 35•8 years ago
|
||
About comment 32, did we end up changing how print preview works with all the pages?
Assignee | ||
Comment 36•8 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #35)
> About comment 32, did we end up changing how print preview works with all
> the pages?
Hi Olli,
On comment 21, when I said "using the wrong browser" I was referring to a clobbering issue we had on print preview code, see here: https://reviewboard.mozilla.org/r/84330/diff/1#index_header
Before the aforementioned fix: after the first interaction (settings changed), we were using the pp browser itself as source to generate new preview versions based on new settings. In this way, pp browser was only a "rendered canvas", there was no mozPrintCallback getting called at all. That's the reason why it was "working".
I say working in quotes because if our source browser is a PDF.js viewer, we would only make use of it when entering on preview, and not when settings get changed (after first interaction). Even though we're sending print events in the wrong order, this would not affect preview because our pp browser (set as source) was not listening to such events.
After the aforementioned fix: source browser would always be the PDF.js viewer, meaning it is necessary to re-render the preview canvas on every interaction. Since we currently have an issue dispatching print events, and PDF.js listens to these events (set as source), we end up reproducing the current bug (blank page -> canvas gets destroyed by PDF.js when receiving unexpected afterprint event -> comment 21).
Our current fix makes sure we dispatch any pending afterprint event (from previous interaction on preview) prior to dispatch a new beforeprint event (for current interaction on preview).
Comment 37•8 years ago
|
||
(In reply to Mike Conley (:mconley) from comment #23)
> (In reply to dalmirdasilva from comment #21)
>
> Holy smokes, what an analysis! Great job!
Seconded!
Comment 38•8 years ago
|
||
Comment on attachment 8824420 [details] [diff] [review]
WIP2: fix dispatch order of print events
I'm going to take the liberty of r+'ing and canceling the r? on mconley so that we can get this landed.
Flags: needinfo?(jwatt)
Attachment #8824420 -
Flags: review?(mconley)
Attachment #8824420 -
Flags: review?(jwatt)
Attachment #8824420 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 39•8 years ago
|
||
I'll land this for you, but I do want to test one specific thing before I do that.
Keywords: checkin-needed
Comment 40•8 years ago
|
||
Did the chance to use different browser affect pdf.js only, or all printed pages?
When I compare before/afterprint event handling in 45esr to nightly, looks like changes to print preview causes new before/afterprint to the original web page.
That was not the original idea of the clone-document-for-printing.
The idea is that we clone the document and then anything could happen to the original document, like it could be unloaded or os.
How does print preview work now if the original page is unloaded during preview and preview settings are changed? Looks like it is all broken now. We end up print preview the newly loaded page.
Testcase is
data:text/html,<script>window.onafterprint = function() { setTimeout("window.location = 'data:text/plain,REPLACED PAGE!'", 0); }</script><pre>INITIAL PAGE</pre>
And then enter print preview and change portrait/landscape.
Comment 41•8 years ago
|
||
I filed Bug 1329220
Assignee | ||
Comment 42•8 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #40)
> Did the chance to use different browser affect pdf.js only, or all printed
> pages?
> When I compare before/afterprint event handling in 45esr to nightly, looks
> like changes to print preview causes new before/afterprint to the original
> web page.
> That was not the original idea of the clone-document-for-printing.
> The idea is that we clone the document and then anything could happen to the
> original document, like it could be unloaded or os.
>
> How does print preview work now if the original page is unloaded during
> preview and preview settings are changed? Looks like it is all broken now.
> We end up print preview the newly loaded page.
> Testcase is
> data:text/html,<script>window.onafterprint = function() {
> setTimeout("window.location = 'data:text/plain,REPLACED PAGE!'", 0);
> }</script><pre>INITIAL PAGE</pre>
>
> And then enter print preview and change portrait/landscape.
Yes, it affects all pages. Should we run this conversation on the bug 1329220?
Comment 43•8 years ago
|
||
In fx49 we used to dispatch one set of beforeprint/afterprint events, and that was unaffected by the number of times the user might switch between portrait and landscape mode. I'm a bit uncomfortable with the fact that while this patch would prevent us dispatching nested events, it will leave us dispatching another set of events every time the user switches between portrait/landscape. I could describe how to fix that, but to speed things up I've just written the (small) changes myself (still attributing you as the patch author, Dalmir). I'd prefer this patch since restoring something closer to the old behavior seems safer if we're to try to get this uplifted to beta.
Comment 44•8 years ago
|
||
Attachment #8824420 -
Attachment is obsolete: true
Attachment #8824595 -
Flags: review?(dholbert)
Comment 45•8 years ago
|
||
Comment on attachment 8824595 [details] [diff] [review]
Dalmir's patch, modified to avoid multiple sets of events
Review of attachment 8824595 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks -- the multiple instances of before/afterprint were part of what scared me about this before. :)
r=me with the following, as discussed on IRC:
::: layout/base/nsDocumentViewer.cpp
@@ +3893,5 @@
> NS_ENSURE_STATE(doc);
>
> // Dispatch 'beforeprint' event and ensure 'afterprint' will be dispatched:
> + // XXX Currently when the user switches between portrait and landscape mode
> + // in print preview we re-enter this function before
Please elaborate slightly on "Currently", and add a comma after "print preview" on this line.
Attachment #8824595 -
Flags: review?(dholbert) → review+
Comment 46•8 years ago
|
||
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/262dd6d7976d
Prevent dispatch of nested or multiple pairs of beforeprint/afterprint events. r=jwatt,dholbert
Updated•8 years ago
|
Attachment #8824595 -
Attachment description: patch, modified to avoid multiple sets of events → Dalmir's patch, modified to avoid multiple sets of events
Attachment #8824595 -
Flags: review+
Comment 47•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment 48•8 years ago
|
||
Comment on attachment 8824595 [details] [diff] [review]
Dalmir's patch, modified to avoid multiple sets of events
Approval Request Comment
[Feature/Bug causing the regression]: bug 962433
[User impact if declined]: print preview of pdfs can be broken
[Is this code covered by automated tests?]: no - hard to test
[Has the fix been verified in Nightly?]: yes, merged to m-i
[Needs manual test from QE? If yes, steps to reproduce]: yes - see comment 0
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: should be fairly low risk
[Why is the change risky/not risky?]: restores old working behavior as much as possible
[String changes made/needed]: none
Attachment #8824595 -
Flags: approval-mozilla-beta?
Attachment #8824595 -
Flags: approval-mozilla-aurora?
Comment 50•8 years ago
|
||
Andrei, one for your team, a bit last minute but this would be nice to uplift if verified.
Flags: needinfo?(andrei.vaida)
Comment 51•8 years ago
|
||
QA Verification: This is to confirm that this issue is fixed on latest nightly (Build ID 20170109030209)
Comment 52•8 years ago
|
||
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #50)
> Andrei, one for your team, a bit last minute but this would be nice to
> uplift if verified.
(In reply to Kanchan Kumari QA from comment #51)
> QA Verification: This is to confirm that this issue is fixed on latest
> nightly (Build ID 20170109030209)
Thank you for picking this up, Kanchan! :)
Flags: needinfo?(andrei.vaida)
Comment 53•8 years ago
|
||
Comment on attachment 8824595 [details] [diff] [review]
Dalmir's patch, modified to avoid multiple sets of events
print preview fix, verified in nightly, aurora52+
Attachment #8824595 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 54•8 years ago
|
||
bugherder uplift |
Comment 55•8 years ago
|
||
Comment on attachment 8824595 [details] [diff] [review]
Dalmir's patch, modified to avoid multiple sets of events
Fix a regression related to print preview and was verified. Beta51+.
Attachment #8824595 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 56•8 years ago
|
||
Looks like this needs to be rebased around bug 1313386 (or that needs uplifting to Beta too).
Flags: needinfo?(jwatt)
Comment 58•8 years ago
|
||
Verified as fixed with Firefox 52 beta 3 under Win 10 64-bit.
Comment 59•7 years ago
|
||
Hi!
I can still reproduce the issue on Win7 x32 on Fx 63 Nightly Build ID:20180719100142 intermittently if after reaching the Print preview screen I switch between Portrait and Landscape mode several time quickly. After a few tries the pages become blank on the Print preview screen, sometimes after closing the Print preview even the original pdf document has some blank pages. I couldn't reproduce the issue on any other OS versions.
Comment 60•7 years ago
|
||
(In reply to laszlo.bialis from comment #59)
> Hi!
>
> I can still reproduce the issue on Win7 x32 on Fx 63 Nightly Build
> ID:20180719100142 intermittently if after reaching the Print preview screen
> I switch between Portrait and Landscape mode several time quickly. After a
> few tries the pages become blank on the Print preview screen, sometimes
> after closing the Print preview even the original pdf document has some
> blank pages. I couldn't reproduce the issue on any other OS versions.
Can you file a new bug?
Flags: needinfo?(laszlo.bialis)
Comment 61•7 years ago
|
||
Filed new Bug 1477018 for Win 7 x32 as requested above.
Flags: needinfo?(laszlo.bialis)
You need to log in
before you can comment on or make changes to this bug.
Description
•