Bug 315932 - [printing] Print selection prints whole file
Summary: [printing] Print selection prints whole file
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 3.6   Edit
Hardware: All All
: P3 normal with 1 vote (vote)
Target Milestone: 3.6.1   Edit
Assignee: Carolyn MacLeod CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 316290 337356 (view as bug list)
Depends on: 297957
Blocks:
  Show dependency tree
 
Reported: 2010-06-07 02:42 EDT by Hajo Czub CLA
Modified: 2011-02-17 05:08 EST (History)
12 users (show)

See Also:
daniel_megert: pmc_approved+
eclipse.felipe: review+
daniel_megert: review+


Attachments
Fix in case no parallel printing takes place (1.41 KB, patch)
2010-06-08 12:03 EDT, Dani Megert CLA
no flags Details | Diff
patch in StyledText (2.18 KB, patch)
2010-06-09 13:49 EDT, Carolyn MacLeod CLA
daniel_megert: review+
Details | Diff
same patch with unused variable removed (2.24 KB, patch)
2010-07-07 10:06 EDT, Carolyn MacLeod CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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. ***