Gecko extends the native Windows print dialog with frame options: - As laid out on screen - the selected frame - each frame separately. Most of the time however, these options are disabled because the document doesn't contain frames. This affects most websites and pretty much every single email in Thunderbird/Seamonkey. The attached screenshot shows the current print dialog. The locale mismatch is a result of using an English Firefox on a German Windows.

This is the print dialog without the frame options.

Design comment: Doesn't discoverability dictate leaving the options present, but disabling them, just as is done now? Technical comment: Your patch almost certainly makes code later in the same function dead; you should find that code and remove it. Tangential request: Do you think you would be able to implement the "missing piece" of bug 629500 (the native Page Setup dialog box)?

(In reply to Zack Weinberg (:zwol) from comment #3) > Design comment: Doesn't discoverability dictate leaving the options present, > but disabling them, just as is done now? You'll discover the frame options when printing a frameset by looking at a print dialog which is larger than usual because it contains those options. On normal webpages (or emails), i.e. almost always, you simply don't need these options. > Technical comment: Your patch almost certainly makes code later in the same > function dead; you should find that code and remove it. The rest of the function only deals with extending the dialog by the frame options, and calls InitializeExtendedDialog. In the nsIPrintSettings::kFrameEnableNone case, I don't want to add these options and initialize them, so I just return true. But in the other cases, kFrameEnableAll and kFrameEnableAsIsAndEach, the function proceeds as before. > Tangential request: Do you think you would be able to implement the "missing > piece" of bug 629500 (the native Page Setup dialog box)? I'm afraid not. I usually hack XUL+JS+CSS...

(In reply to Steffen Wilberg from comment #4) > > Technical comment: Your patch almost certainly makes code later in the same > > function dead; you should find that code and remove it. > The rest of the function only deals with extending the dialog by the frame > options, and calls InitializeExtendedDialog. In the > nsIPrintSettings::kFrameEnableNone case, I don't want to add these options > and initialize them, so I just return true. Looking at the code, InitializeExtendedDialog can no longer be called with kFrameEnableNone, which means this block should be removed: 579 } else { // nsIPrintSettings::kFrameEnableNone 580 // we are using this function to disabe the group box 581 SetRadio(hdlg, grp3, PR_FALSE, PR_FALSE); 582 // now disable radiobuttons 583 SetRadio(hdlg, rad4, PR_FALSE, PR_FALSE); 584 SetRadio(hdlg, rad5, PR_FALSE, PR_FALSE); 585 SetRadio(hdlg, rad6, PR_FALSE, PR_FALSE); 586 } and replaced with an NS_ABORT_IF_FALSE. There may be other such blocks elsewhere in the file; I didn't see any, but I'm not certain.

OK, I've updated the other caller of InitializeExtendedDialog as well, so I could remove the code block you mentioned. That other caller is PropSheetCallBack, which is dead code, inside #ifdef MOZ_REQUIRE_CURRENT_SDK, only used in this file and defined nowhere. I think it's about the more modern Windows print dialog.

Comment on attachment 552790 [details] [diff] [review] patch v1.1 (In reply to Steffen Wilberg from comment #6) > Created attachment 552790 [details] [diff] [review] > patch v1.1 > > OK, I've updated the other caller of InitializeExtendedDialog as well, so I > could remove the code block you mentioned. Thanks. Looks good. > That other caller is PropSheetCallBack, which is dead code, inside #ifdef > MOZ_REQUIRE_CURRENT_SDK, only used in this file and defined nowhere. ... and it's been that way since the beginning of _CVS_ history. Oh my aching head. I am under the impression that we have required a fairly recent Windows SDK for some time, so it might be safe to enable that code unconditionally, but we probably need to have a conversation about whether it's worth doing, and it should happen in a new bug. > I think it's about the more modern Windows print dialog. That does appear to be what it is about, to my limited understanding of Windows.

Comment on attachment 552790 [details] [diff] [review] patch v1.1 > // Special Hook Procedure for handling the print dialog messages > static UINT CALLBACK PrintHookProc(HWND hdlg, UINT uiMsg, WPARAM wParam, LPARAM lParam) > { >+ return TRUE; TRUE is a UINT now?

(In reply to Ms2ger from comment #11) I followed the existing code. "return TRUE" was added at the end of PrintHookProc in bug 628157. See bug 628157 comment 9: > The documentation for WM_INITDIALOG says that true should be returned > for focus to be set.