Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 315932

Summary: [printing] Print selection prints whole file
Product: [Eclipse Project] Platform Reporter: Hajo Czub <Hajo>
Component: SWTAssignee: Carolyn MacLeod <carolynmacleod4>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: aleherb+eclipse, angvoz.dev, carolynmacleod4, ccc, coegig, daniel_megert, deepakazad, eclipse.felipe, keely_tigs, nsand.dev, pwebster, thatnitind
Version: 3.6Flags: daniel_megert: pmc_approved+
eclipse.felipe: review+
daniel_megert: review+
Target Milestone: 3.6.1   
Hardware: All   
OS: All   
Whiteboard:
Bug Depends on: 297957    
Bug Blocks:    
Attachments:
Description Flags
Fix in case no parallel printing takes place
none
patch in StyledText
daniel_megert: review+
same patch with unused variable removed none

Description Hajo Czub CLA 2010-06-07 02:42:58 EDT
Build Identifier:  I20100520-1744

If you open a xmxl file, select some lines , press <CTRL>+P and select "Selected Text" at the printer dialog, the hole xml file will be printed.
This is especially annoying if you print at a network printer (in separate room) and the xml file has a size of about 2MB.

Reproducible: Always

Steps to Reproduce:
1. open a xml file
2. select several lines
3. goto print and select "Selection"
Comment 1 Rakesh CLA 2010-06-08 06:36:00 EDT
It appears to be a problem in org.eclipse.jface.text.TextViewer.It is happening in general text file also.
Comment 2 Nick Sandonato CLA 2010-06-08 10:22:11 EDT
Redirecting to platform based on Rakesh's observations.
Comment 3 Dani Megert CLA 2010-06-08 11:43:30 EDT
Regression compared to 3.5.2.

For a proper fix we need bug 297957 to get fixed, otherwise we need to block printing in parallel.
Comment 4 Dani Megert CLA 2010-06-08 12:03:37 EDT
Created attachment 171425 [details]
Fix in case no parallel printing takes place

This patch fixes the case where no parallel printing is done. Blocking parallel printing in 3.6.1 is no option.
Comment 5 Anton Leherbauer CLA 2010-06-09 10:01:05 EDT
*** Bug 316290 has been marked as a duplicate of this bug. ***
Comment 6 Dani Megert CLA 2010-06-09 13:19:57 EDT
Can be fixed in SWT, see bug 297957 comment 19.
Comment 7 Carolyn MacLeod CLA 2010-06-09 13:49:41 EDT
Created attachment 171558 [details]
patch in StyledText
Comment 8 Carolyn MacLeod CLA 2010-06-09 13:54:12 EDT
Felipe, this fix should go into 3.6.1. The patch is in StyledText.Printing, and it is very simple. I can walk you through the reason it is needed.
Dani, can you review also, please?
Comment 9 Felipe Heidrich CLA 2010-06-10 11:01:37 EDT
The patch is fine.

So, is the application (indirectly, via Printer#getData()) sharing the PrinterData object (and changing the scope) between jobs (running in different threads) ?
Comment 10 Felipe Heidrich CLA 2010-06-10 11:06:24 EDT
Note: if it is allowed the share PrintData objects between Printers (which are allowed to run in different threads).

Then the right fix is in Printer, it needs to save a copy of the PrinterData object passed in the constructor. This only way Printer#getData() will always return the PrinterData object with the "correct/unchanged" data in it.
Other clients can have the same bug as StyledText.
Comment 11 Dani Megert CLA 2010-06-11 05:53:25 EDT
Regarding comment 10: yes you're right but the main problem is that the Javadoc doesn't say anything clearly when it comes to printing:
- Does Printer take a copy or fetch most recent values from PrinterData?
- Does PrintDialog.open() return a new object or the one set in setPrinterData?
- Do the setter methods modify my object that I provided via setPrinterData?


The attached patch completes the existing code pattern in org.eclipse.swt.custom.StyledText.Printing and it fixes our case because before printing we always go via PrintDialog, which - though not clearly specified - always returns a new PrinterData instance. For 3.6.1 this is definitely the right fix. For 3.7 the Javadoc should be clarified and implemented accordingly.

Note that the following line is no longer needed and should be removed:
#442: PrinterData data = printer.getPrinterData();

I also verified that the patch fixes our problem.

+1 for 3.6.1.


BTW: Could you mention in the Javadoc of PrintDialog.open() that it always returns a new object? Thanks! ;-)
Comment 12 Carolyn MacLeod CLA 2010-06-11 12:24:52 EDT
I made a note in bug 297957 to make sure that the implementation and the doc are consistent. I am reluctant to change the PrintDialog.open doc for 3.6.1 because it may change in 3.7. Please make any further comments in that bug.
Comment 13 Carolyn MacLeod CLA 2010-06-11 14:51:30 EDT
Oh, by the way, re comment 9:
> So, is the application (indirectly, via Printer#getData()) sharing the
> PrinterData object (and changing the scope) between jobs (running in different
> threads) ?

yep  :)
Comment 14 Carolyn MacLeod CLA 2010-07-07 10:06:04 EDT
Created attachment 173654 [details]
same patch with unused variable removed

This patch is the same as the one above, with the following line removed:
PrinterData data = printer.getPrinterData();
(it was originally needed for data.scope, but it is no longer needed because scope is cached in the constructor).
Comment 15 Carolyn MacLeod CLA 2010-07-07 10:08:00 EDT
Fixed > 20100707 in 3.7 (HEAD).
Comment 16 Carolyn MacLeod CLA 2010-07-07 10:11:24 EDT
Fixed > 20100707 in 3.6.1 branch.
Comment 17 Dani Megert CLA 2010-07-08 01:56:49 EDT
Verified for 3.7 in N20100707-2000.
Comment 18 Dani Megert CLA 2011-02-17 05:08:23 EST
*** Bug 337356 has been marked as a duplicate of this bug. ***