Closed Bug 965991 Opened 9 years ago Closed 7 years ago

nsPrintSettingsGTK copy constructor missed initializing paper size

Categories

(Core :: Widget: Gtk, defect)

26 Branch
x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1227008

People

(Reporter: WeirdAl, Assigned: WeirdAl)

Details

(Keywords: crash)

Attachments

(2 files)

Sample code:
const Ci = Components.interfaces;
var wbPrint = getBrowser().contentWindow
                          .QueryInterface(Ci.nsIInterfaceRequestor)
                          .getInterface(Ci.nsIWebBrowserPrint);
var settings = wbPrint.globalPrintSettings.clone();
wbPrint.print(settings, this); // CRASH

(gdb) print mPaperSize
$1 = (GtkPaperSize *) 0xa5a5a5a5a5a5a5a5
From a quick look the operator= should create copies for all internals/add references for the gtk objects, that should be called by the second constructor, that is called by _Clone.
If anyone thinks they have the time to develop a fix for this bug, go for it; it'll be some time before I can find the cycles.
Comment on attachment 8690220 [details] [diff] [review]
Copy constructor support for paperName in nsPrintSettingsGTK

I hope this is right, or close to it.
Attachment #8690220 - Flags: review?(karlt)
Comment on attachment 8690220 [details] [diff] [review]
Copy constructor support for paperName in nsPrintSettingsGTK

>+  {
>+    nsPrintSettingsGTK& other = const_cast<nsPrintSettingsGTK&>(rhs);
>+    char16_t* paperName;
>+    other.GetPaperName(&paperName);
>+    SetPaperName(paperName);
>+    NS_Free(paperName);
>+  }
>+

mPageSetup already has a suitable GtkPaperSize, so all that is needed
here is to gtk_paper_size_free(mPaperSize), and set mPaperSize from
gtk_paper_size_copy(gtk_page_setup_get_paper_size()).

(I'm not sure that the paperName always describes all of the info in GtkPaperSize).

> nsresult nsPrintSettingsGTK::_Clone(nsIPrintSettings **_retval)

>+  char16_t* paperName;
>+  GetPaperName(&paperName);
>+  newSettings->SetPaperName(paperName);
>+  NS_Free(paperName);
>+

The copy constructor (used by Clone()) uses operator=, so no need to do
anything in Clone() because fixing operator= is enough to fix this.
Attachment #8690220 - Flags: review?(karlt) → review-
Component: Printing: Output → Widget: Gtk
The patch in bug 1227008 fixes this by removing mPaperSize altogether.
Depends on: 1227008
See Also: 1227008
Status: NEW → RESOLVED
Closed: 7 years ago
No longer depends on: 1227008
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.