Closed Bug 717178 Opened 13 years ago Closed 12 years ago

Printing stops after 73%, and browser hangs up

Categories

(Core :: Graphics, defect)

Product:
Core
Component:
Graphics
Version:
9 Branch
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla21
Tracking Flags:
Tracking Status
firefox19 --- affected
firefox20 + verified
firefox21 --- verified

People

(Reporter: franz.ehrenhuber, Assigned: roc)

References

(Blocks 1 open bug)

Details

(Keywords: hang, regression)

Attachments

(4 files, 4 obsolete files)

Html.zip
76.18 KB, application/octet-stream
Details
Part 1: Don't crash when passing a nil scaled-font to _name_tables_match. r=jfkthame
1.10 KB, patch
jfkthame
: review+
bajaj
: approval-mozilla-aurora+
Details | Diff | Splinter Review
part 2 v2
11.83 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
alternative to parts 3 and 4, v2, for checkin
11.91 KB, patch
bajaj
: approval-mozilla-aurora+
Details | Diff | Splinter Review
Attached file Html.zip —
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0.1) Gecko/20100101 Firefox/9.0.1 Build ID: 20111220165912 Steps to reproduce: Printing a local html page. Actual results: Printing stops after 73% and firefox is frozen Expected results: Printing continues to 100% and document is sent to the printer
I have added the html file used to do printing and a screenshot about last status when firefox stops working
Regression window(m-c) Works: http://hg.mozilla.org/mozilla-central/rev/5faaa652594f Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110915 Firefox/9.0a1 ID:20110915013748 Fails: http://hg.mozilla.org/mozilla-central/rev/c6a405d01a1d Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110915 Firefox/9.0a1 ID:20110915064148 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5faaa652594f&tochange=c6a405d01a1d Regression window(m-i) Works: http://hg.mozilla.org/integration/mozilla-inbound/rev/7e132eee5650 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110914 Firefox/9.0a1 ID:20110914103830 Fails: http://hg.mozilla.org/integration/mozilla-inbound/rev/c614dabf94e2 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110914 Firefox/9.0a1 ID:20110914115129 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=7e132eee5650&tochange=c614dabf94e2 Suspected: fec42fae34a7 Adrian Johnson — Bug 454532. cairo: Print as Unicode. r=jrmuizel
Blocks: 454532
Status: UNCONFIRMED → NEW
Component: Untriaged → Graphics
Ever confirmed: true
Keywords: hang, regression
Product: Firefox → Core
QA Contact: untriaged → thebes
Hardware: x86_64 → x86
It works properly if I disable HWA
Severity: normal → critical
Summary: Printing stops after 73% → Printing stops after 73%, and browser hangs up
Just verified that. If I disable HWA without closing the browser and printing again, FF crashes. After restart, printing works fine. But there seems to be a seriuos problem, with HWA and printing. I think you can not tell the user, 'If you want to print then disable HWA' This should work anyway!
Adrian, can you take a look at this?
I can't reproduce the bug.
Reproduced with: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0.1) Gecko/20100101 Firefox/9.0.1 Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:12.0a1) Gecko/20120111 Firefox/12.0a1
No longer blocks: 454532
This only occurs with the DirectWrite fontlist code. Bug 454532 was a change to the GDI fontlist code, so it's not possible that that bug caused this. I think this is a combination of an error in our printing code that is somehow causing the hang detection code to put the event handling code in a big spin loop. Running this in the debugger, that appears to be whats happening but event handling code is a bit of a mystery to me so I can't assert this for sure.
Loop in printing code: The code in nsPrintEngine::PrintPage calls nsSimplePageSequenceFrame::DoPageEnd: http://mxr.mozilla.org/mozilla-central/source/layout/printing/nsPrintEngine.cpp#2521 The code in this method executes: rv = PresContext()->DeviceContext()->EndPage(); This fails but the error returned is ignored by the calling method, PrintPage. An infinite loop results.
Argh, missed one of the changesets on that bug. Probable cause (from bug 454532): https://hg.mozilla.org/mozilla-central/rev/fec42fae34a7
Blocks: 454532
Confirmed that with the changeset just before the one noted in comment 10, the testcase attached here prints out completely. After the patch, the UI stalls at 67% and only 11 out of 18 pages print (with default page size set to A4). The curious thing is that before the change was landed, the progress dialog appears only briefly. After the change it appears for several seconds before the progress hits 67% and stalls.
I can reproduce this by setting gfx.direct2d.force-enabled to true (my laptop has Intel 965 graphics). I don't know how to get the debugger to work with firefox so I can't see exactly where it is failing. However the most useful clue is the fact it only occurs when using DirectWrite fonts. Looking at how the win32 printing surface handles dwrite fonts I see that _cairo_dwrite_scaled_font_create_win32_scaled_font() is being called on every call to cairo_show_glyphs() to create a new win32 font face and scaled font. This font face and scaled font are not destroyed at the end of the show_glyphs call because a reference is held by the win32 printing surface until the page is finished. It is possible that after creating too many HFONT objects GDI calls start failing. There are some patches in cairo git master to use a hash table of win32 font faces to avoid creating a new font face if there is already an existing font face with the same LOGFONT. After applying the following patches from cairo git master: eb29a25d 6e3e3291 101fab7c The test case now works for me.
Jeff, is there a cairo update plan that would include the patches Adrian mentions?
Blocks: 751421
I had the same problem, solved by disabled HWA. I did a lot of testing before I found the solution and found the exact document size that would cause this failure to occur: 106187 OK, 106188 ERROR. This was on a different document that unfortunately I cannot share. When printing to file, the resulting size when the error occurs is 716 kb.
I have the same issue . My page will be 33 pages after print but it is waiting on page 12 then I delete on going print queue it will print that 12 pages but firefox freeze and I have to close and reopen it.
Jeff, do you think you could find someone to look at this?
(In reply to John Daggett (:jtd) from comment #9) > Loop in printing code: > > The code in nsPrintEngine::PrintPage calls > nsSimplePageSequenceFrame::DoPageEnd: > > http://mxr.mozilla.org/mozilla-central/source/layout/printing/nsPrintEngine. > cpp#2521 > > The code in this method executes: > > rv = PresContext()->DeviceContext()->EndPage(); > > This fails but the error returned is ignored by the calling method, > PrintPage. An infinite loop results. Need to fix this one sure, but also the printing problem.
Blocks: 802476
Assignee: nobody → roc
Thanks Adrian! These patches fix the testcase for me too.
Comment on attachment 672685 [details] [diff] [review] Part 1: Don't crash when passing a nil scaled-font to _name_tables_match. r=jfkthame Review of attachment 672685 [details] [diff] [review]: ----------------------------------------------------------------- I guess this happens when windows fails to create the new GDI font resource? I don't think it should be possible for the "original" dwrite font to have a nil backend here, as we've already been accessing it before getting to this point. Still, the simplest and safest fix here is just to check both fonts, rather than making (fragile) assumptions about which of them is "known" to be good.
Attachment #672685 - Flags: review?(jfkthame) → review+
Attachment #672686 - Flags: review?(jmuizelaar) → review+
Comment on attachment 672685 [details] [diff] [review] Part 1: Don't crash when passing a nil scaled-font to _name_tables_match. r=jfkthame [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 802476 User impact if declined: crashes when printing Testing completed (on m-c, etc.): none yet Risk to taking this patch (and alternatives if risky): this patch is incredibly low risk, just a couple of null checks. No real alternatives. String or UUID changes made by this patch: none.
Attachment #672685 - Flags: approval-mozilla-aurora?
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #26) > Bug caused by (feature/regressing bug #): bug 802476 This is wrong, it's actually bug 468568.
Part 2 caused test failures on Windows XP so I backed it out. https://tbpl.mozilla.org/php/getParsedLog.php?id=16366499&tree=Mozilla-Inbound&full=1 It looks like GDI text rendering just stopped working.
Whiteboard: [leave open]
REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bidi/263359-1b.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/40596-1h.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/180085-1.html | image comparison (==), max difference: 255, number of differing pixels: 3042 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/180085-2.html | image comparison (==), max difference: 255, number of differing pixels: 2366 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/296361-1.html | image comparison (==), max difference: 255, number of differing pixels: 30 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/315920-1c.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/315920-1d.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/315920-3e.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/315920-3f.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/338251-pre.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/341043-1b.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/350506-1.html | image comparison (==), max difference: 255, number of differing pixels: 18 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/411059-1.html | image comparison (==), max difference: 255, number of differing pixels: 19608 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/455826-1.html | image comparison (==), max difference: 255, number of differing pixels: 194 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/475986-1-ref.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/475986-1-ref.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/475986-2-ref.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/css-placeholder/input/placeholder-add.html | image comparison (==), max difference: 128, number of differing pixels: 111 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/css-placeholder/input/placeholder-value-unset.html | image comparison (==), max difference: 128, number of differing pixels: 111 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/css-placeholder/input/placeholder-value-reset.html | image comparison (==), max difference: 128, number of differing pixels: 111 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/css-placeholder/input/placeholder-type-change-1.html | image comparison (==), max difference: 128, number of differing pixels: 111 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/flexbox/flexbox-float-1c.xhtml | image comparison (==), max difference: 238, number of differing pixels: 47 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/flexbox/flexbox-float-1d.xhtml | image comparison (==), max difference: 238, number of differing pixels: 47 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/font-face/local-styled-1.html | assertion count 2 is more than expected 0 assertions REFTEST TEST-UNEXPECTED-FAIL | view-source:file:///C:/talos-slave/test/build/reftest/tests/parser/htmlparser/tests/reftest/bug482921-1.html | image comparison (==), max difference: 255, number of differing pixels: 18 REFTEST TEST-UNEXPECTED-FAIL | view-source:file:///C:/talos-slave/test/build/reftest/tests/parser/htmlparser/tests/reftest/bug482921-2.xhtml | image comparison (==), max difference: 255, number of differing pixels: 18 REFTEST TEST-UNEXPECTED-FAIL | view-source:file:///C:/talos-slave/test/build/reftest/tests/parser/htmlparser/tests/reftest/bug700260-1.html | image comparison (==), max difference: 255, number of differing pixels: 432 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/tab-size/tab-size-8.html | image comparison (==), max difference: 255, number of differing pixels: 126 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/tab-size/tab-size-4.html | image comparison (==), max difference: 255, number of differing pixels: 54 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/tab-size/tab-size-4-span.html | image comparison (==), max difference: 255, number of differing pixels: 54 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/tab-size/tab-size-4-spanoffset.html | image comparison (==), max difference: 255, number of differing pixels: 216 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/tab-size/tab-size-4-multiple.html | image comparison (==), max difference: 255, number of differing pixels: 72 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/tab-size/tab-size-1.html | image comparison (==), max difference: 255, number of differing pixels: 18 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-textarea-attr.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-textarea-nofocus.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-textarea-disabled.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-textarea-attr-inherit.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-textarea-attr-dynamic.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-textarea-attr-dynamic-inherit.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-textarea-property-dynamic.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-textarea-property-dynamic-inherit.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-textarea-attr-dynamic-override.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-textarea-attr-dynamic-override-inherit.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-textarea-property-dynamic-override.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-textarea-property-dynamic-override-inherit.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-hyphen-invalid.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-hyphen-multiple-invalid.html | image comparison (!=) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/editor/reftests/spellcheck-superscript-2.html | image comparison (!=)
Interestingly, disabling D2D on Windows 7 doesn't seem to exhibit the bug.
Waiting on the test failures to get resolved before approving on Aurora .
I can't reproduce in my Windows XP VM either.
Attachment #672685 - Flags: approval-mozilla-aurora?
Comment on attachment 672685 [details] [diff] [review] Part 1: Don't crash when passing a nil scaled-font to _name_tables_match. r=jfkthame [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 802476 User impact if declined: crashes when printing Testing completed (on m-c, etc.): patch on central for a day Risk to taking this patch (and alternatives if risky): this patch is incredibly low risk, just a couple of null checks. No real alternatives. The other patch in this bug caused test failures, but this patch did not and is on central now and should fix the crash regressions. String or UUID changes made by this patch: none.
Attachment #672685 - Flags: approval-mozilla-aurora?
Comment on attachment 672685 [details] [diff] [review] Part 1: Don't crash when passing a nil scaled-font to _name_tables_match. r=jfkthame Approving this patch for Aurora as it fixes crashes when printing
Attachment #672685 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Looks to me like this landed in 19, should it be marked fixed for that?
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #28) > Part 2 caused test failures on Windows XP so I backed it out. > > https://tbpl.mozilla.org/php/getParsedLog.php?id=16366499&tree=Mozilla- > Inbound&full=1 > > It looks like GDI text rendering just stopped working. I guess that's why it's still open, is that still to be addressed?
I can still reproduce the problem of comment #0 in Firefox18beta,19.0a2 and 20.0a1. http://hg.mozilla.org/releases/mozilla-beta/rev/6eb5edeb9f52 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0 ID:20121128060531 http://hg.mozilla.org/releases/mozilla-aurora/rev/8e070b999a77 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20121202 Firefox/19.0 ID:20121202042013 http://hg.mozilla.org/mozilla-central/rev/0352a32fde64 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121202 Firefox/20.0 ID:20121202030723
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #41) > I guess that's why it's still open, is that still to be addressed? Yeah, that's right.
I don't understand how cairo_font_face_t lifetime management works with this cairo patch. We call cairo_win32_font_face_create_for_logfontw_hfont. It creates a hashtable entry. What ever removes that entry from the hashtable? As far as I can tell, nothing ever does, even if the cairo_win32_font_face_t dies.
That's easy to fix by removing the cairo_win32_font_face_t from the hashtable when it's destroyed. But we seem to have another problem, in gfxGDIFont's destructor. We unreference the cairo_win32_font_face_t and then call DeleteObject on mFont. But we don't actually know whether the cairo_win32_font_face_t was deleted; it may still be alive and in use somewhere (perhaps due to being returned from the hashtable by a different call by cairo_win32_font_face_create_for_logfontw_hfont). If it's still in use, then the call to DeleteObject on gfxGDIFont::mFont means the cairo_win32_font_face_t's mFont is bogus and later attempts to use it will fail.
So basically right now there is no guarantee that the HFONT passed to cairo_win32_font_face_create_for_logfontw_hfont will remain alive as long as it is needed by the cairo_font_face_t we return.
Build failure due to some symbol export thing. Re-pushed: https://tbpl.mozilla.org/?tree=Try&rev=726cf95974f7
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #47) > But we seem to have another problem, in gfxGDIFont's destructor. We > unreference the cairo_win32_font_face_t and then call DeleteObject on mFont. > But we don't actually know whether the cairo_win32_font_face_t was deleted; You call cairo_font_face_set_user_data() after creating the font_face to set the function to be called when the font_face is destroyed.
You mean we could, but we currently don't :-). Yes, we could do that. But it seems silly to make that a requirement for using cairo_win32_font_face_create_for_logfontw_hfont safely.
Attached patch part 2 v2 — — Splinter Review
Updated to remove the cairo_font_face_t from the hashtable when it is destroyed.
Attachment #672686 - Attachment is obsolete: true
Attachment #689625 - Flags: review?(jmuizelaar)
This is an API change I guess, but the previous API was nigh impossible to use correctly.
Attachment #689626 - Flags: review?(jmuizelaar)
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #54) > This is an API change I guess, but the previous API was nigh impossible to > use correctly. If this API change is deemed unacceptable, I'll rework the patch to use the user_data approach.
Attachment #689625 - Flags: review?(jmuizelaar) → review+
Comment on attachment 689626 [details] [diff] [review] Part 3: Pass ownership of HFONT to the cairo_scaled_font_t Review of attachment 689626 [details] [diff] [review]: ----------------------------------------------------------------- Sorry this took so long.
Attachment #689626 - Flags: review?(jmuizelaar) → review+
Attachment #696854 - Flags: review?(jmuizelaar) → review+
The mochitest failure shows a real bug. The problem is that with these patches, gfxGDIFont's mFont is no longer reliable since if we match a cached cairo_font_face_t, we'll delete the passed-in HFONT immediately. But the dead mFont is still stored in gfxGDIFont, and used in various places afterwards. We could try to fix that, but I think it might be better to reduce code changes here and simply not cache anything where we had a passed-in HFONT. The cache setup simply doesn't work well in that case.
Attached patch alternative to parts 3 and 4 (obsolete) — — Splinter Review
This is green on try. I still think the cairo APIs that take an HFONT are broken; it's just bad to force the caller to ensure that all references to the refcounted cairo_font_face_t are dropped (including references via scaled fonts, caches, etc) before the caller destroys the HFONT. However, fixing that API issue seems like more trouble than it's worth.
Attachment #696977 - Flags: review?(jmuizelaar)
Comment on attachment 696977 [details] [diff] [review] alternative to parts 3 and 4 Review of attachment 696977 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/cairo/cairo/src/cairo-win32-font.c @@ +1,1 @@ > +/* -*- Mode: c; tab-width: 8; c-basic-offset: 4; indent-tabs-mode: t; -*- */ ?
Attachment #696977 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/mozilla-central/rev/ef465cf5fc12 https://hg.mozilla.org/mozilla-central/rev/1435e8a04081
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Printing not working is pretty bad.
status-firefox19: --- → affected
tracking-firefox19: --- → ?
Actually, don't bother tracking for 19. This regressed in Firefox 9 so it's not worth putting a fix into beta.
tracking-firefox19: ? → ---
Comment on attachment 698801 [details] [diff] [review] alternative to parts 3 and 4, v2, for checkin [Approval Request Comment] Bug caused by (feature/regressing bug #): 454532 User impact if declined: Printing some pages fails; printing other pages might consume lots of memory Testing completed (on m-c, etc.): just landed Risk to taking this patch (and alternatives if risky): a bit risky, so requesting only Aurora approval String or UUID changes made by this patch: none
Attachment #698801 - Flags: checkin? → approval-mozilla-aurora?
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #71) > Comment on attachment 698801 [details] [diff] [review] > alternative to parts 3 and 4, v2, for checkin > > [Approval Request Comment] > Bug caused by (feature/regressing bug #): 454532 > User impact if declined: Printing some pages fails; printing other pages > might consume lots of memory > Testing completed (on m-c, etc.): just landed > Risk to taking this patch (and alternatives if risky): a bit risky, so > requesting only Aurora approval > String or UUID changes made by this patch: none roc, considering the risk profile here and based on the patch do we know the severity of regressions this may cause? This has been there since Fx9, do you think letting this ride the trains would be of any help here in terms of getting more testing ?
(In reply to bhavana bajaj [:bajaj] from comment #72) > roc, considering the risk profile here and based on the patch do we know the > severity of regressions this may cause? The severity is that some pages (probably any sufficiently long and complex page) can't be printed in Firefox on Windows 7. You can see there are six duplicates filed. > This has been there since Fx9, do > you think letting this ride the trains would be of any help here in terms of > getting more testing ? It would, of course. I think the latest patches are pretty low-risk --- we bypass the new caching code for almost all our font usage.
Comment on attachment 698801 [details] [diff] [review] alternative to parts 3 and 4, v2, for checkin Looks like this hasn't landed yet. Should this be reopened?
AFAICT it's landed on inbound/m-c (1435e8a04081, the second changeset in comments 67-68), but awaiting approval for aurora.
Comment on attachment 698801 [details] [diff] [review] alternative to parts 3 and 4, v2, for checkin Approving, the alternative low risk patches for aurora. We have also requested QA testing/verification on this issue .
Attachment #698801 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Depends on: 838617
Is it too late to get this patched in 19? My users have encountered this problem, loaded up Aurora on my pc, and it fixed it, but I'd rather not have Aurora on the user's computers.
I think it is, sorry. This bug has been around for over a year so the danger of ramming it into FF19 doesn't seem justified by getting it fixed six weeks earlier.
Reproduced on FF 19. Verified fixed Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0b1
status-firefox20: fixed → verified
Keywords: verifyme
QA Contact: paul.silaghi
Verified fixed Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20100101 Firefox/21.0b1
Status: RESOLVED → VERIFIED
status-firefox21: fixed → verified
Keywords: verifyme
I can reproduce this issue with Firefox 17.0.9 ESR on Win 7 64bit, stopping at 96%. Does anyone have any thoughts/suggestions? Thanks!
That's expected, AFAICS. This was fixed for Firefox 20, and was never nominated for backporting to ESR-17.
It would be nice to have this on ESR too. Meanwhile, large files can't be printed.
If it's sufficiently important, you could nominate it for esr tracking/uplift -- though assuming esr24 will be appearing "real soon now" (next week?), I think it's questionable whether it would be backported to esr17 at this point. Perhaps more important would be to verify that the problem does *not* occur on esr24, as that's the version we're about to ship.
Yes, ESR 24 is shipping next week. I don't think anyone will approve backporting this to ESR 17 at this point. People who need this should update to ESR 24 as soon as they can.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: