146399 - Remove unneeded event code

Status: RESOLVED FIXED

(Core :: Print Preview, defect, P1)

Description

[rods (gone)]

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

Comment 1

[Simon Montagu :smontagu]

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

Comment 2

[Vidar Haarr (not reading bugmail)]

Attachment: version 0.1

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 3

[Vidar Haarr (not reading bugmail)]

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 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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).

Comment 5

[Vidar Haarr (not reading bugmail)]

(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.

Comment 6

[Boris Zbarsky [:bzbarsky]]

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

Comment 7

[Vidar Haarr (not reading bugmail)]

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

Filed bug 309754.

Comment 8

[Vidar Haarr (not reading bugmail)]

Attachment: version 0.2

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.

Comment 9

[Boris Zbarsky [:bzbarsky]]

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 10

[Vidar Haarr (not reading bugmail)]

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 11

[rbs]

Comment on attachment 197398 [details] [diff] [review]
version 0.2

sr=rbs

Comment 12

[Boris Zbarsky [:bzbarsky]]

Checked in