When bug 124990 is fixed we need to back out the changes put in for Bug 129002

Bug 124990 was fixed over a year ago. Maybe the time has come?

I was poking around the code about another bug and came across these comments, so I thought I'd just remove the lines and add a patch here. Please note that I do not have a printer, so I can't test this code at all.

Comment on attachment 196771 [details] [diff] [review] version 0.1 I compiled seamonkey (which lets you print preview even if you don't have a printer, using the postscript output module), and from what I can gather things work fine. I don't know who can review this code. Everyone involved in bug 129002 is gone. I see dbaron made a comment there, so requesting review from him. dbaron: I think the entire HandleEvent method can be removed from nsTextControlFrame.cpp, but it didn't come to mind when producing the patch. Want me to remove it completely?

Comment on attachment 196771 [details] [diff] [review] version 0.1 If you've tested that bug 129002 is still fixed, r=dbaron. And yes, please do remove the whole function in nsTextControlFrame (and don't forget the .h file change).

(In reply to comment #4) > (From update of attachment 196771 [details] [diff] [review] [edit]) > If you've tested that bug 129002 is still fixed, r=dbaron. And yes, please do > remove the whole function in nsTextControlFrame (and don't forget the .h file > change). When I tried the patch in seamonkey as noted in comment #3, it worked fine. A clean unmodified CVS tree from today and trying Print Preview (even on a blank page) freezes the browser, so I can't really test again. Unfortunately I don't have time to investigate further right now, but it seems we have a regression somewhere.

Which regression? Please file a bug on it, with steps to reproduce?

(In reply to comment #6) > Which regression? Please file a bug on it, with steps to reproduce? Filed bug 309754.

The 'regression' was actually some problem in Ubuntu. Updated patch removes nsTextControlFrame::HandleEvent entirely. I've tested the patch by print previewing various forms (including a bugzilla form) and the testcase in bug 129002, and I'm unable to produce any crashes.

I won't get to this for a while (possibly weeks). You may want to seek a different sr; there are lots of them out there!

Comment on attachment 197398 [details] [diff] [review] version 0.2 (In reply to comment #9) > I won't get to this for a while (possibly weeks). You may want to seek a > different sr; there are lots of them out there! Indeed there is, but you were also CC'ed and have a comment or two in bug 129002 :-)

Comment on attachment 197398 [details] [diff] [review] version 0.2 sr=rbs

Checked in