Created attachment 47221 [details]
Reverses page order in Page Preview for RTL

MRU->HI: please sent this PATCH issue to responsible developer. Thanks!

HI->OD: Please evaluate.

review of patch in progress.

OD->ayaniger:
Reviewing the patch reveals the following insufficiencies:
(a) The shown pages are no longer horizontal centered. To reproduce, switch to
the print preview and decrease the zoom factor. You will see that the shown
pages are left aligned instead of horizontal centered.
(b) After zooming in (increase the zoom factor) the print preview doesn't
function as expected.
(c) In the Print Preview a certain page is selected. The selection is marked by
default by a blue border. The selected page can be changed using the arrow-keys
on your keyboard. Left arrow-key and right arrow key doesn't work as expected.

Proposed solution for (a) and (b):
Perform your adjustment of the page position in method
<SwPagePreviewLayout::_CalcPreviewPages()>.
Hint: In this method the local variable <aCurrPaintOffset> holds the anchor
position in the "print preview page grid" for each page. Instead of running the
X-value from initial X to (initial X + (column count - 1) * column width), let
the X-value run from (initial X + (column count - 1) * column width) to initial X

Proposed solution for (c):
Horizontal move requests have to be toggled in method
<SwPagePreviewLayout::CalcStartValuesForSelectedPageMove(..)

Thus, please adjust your patch accordingly.

Another question is, should the print preview layout direction depend on the
environment (Application::GetSettings().GetLayoutRTL()) or on the concrete
layout direction of the document?
Currently, I can't answer this question. Does it makes sense, that a document in
LTR layout is laid out in the print preview in RTL, because the environment is
RTL (or the other way: RTL document in LTR environment)? Or are these cases too
rare respectively out of scope?

ayaniger->OD:

Could you review this revised patch?

Regarding your points:
a) I've fixed the horizontal centering
b) zooming behaves as in the distributed version
c) arrow keys work as expected
d) print preview layout direction depends on the concrete layout direction of
the document

Thank you.

Created attachment 47950 [details]
Revised patch

OD->ayaniger:
Thanks for your revised patch. I will review it at the end of the week or at the
beginning of the next week. Stay tuned.
I'll take over this patch issue for the review.

OD->ayaniger:
Create work. Patch looks good. I've made some minor adjustments - see coming
attachment.

But, we forgot the print of the print preview - method
ViewShell::PrintPreViewPage(..) needs adjustments for Right-to-Left layout.
Please provide a corresponding patch for the print of the print preview as well
- assigning issue back to you.

Created attachment 48105 [details]
revised patch with minor adjustments

some comment on my minor adjustments to your provided patch:
- naming of member variable <bRTL> -> <mbRTL>
- <mbRTL> is now initialized in the constructor and it is set depending on the
layout direction of the first page of the text document.

forgot to assign back to ayaniger ;-)

Created attachment 48137 [details]
handles RTL in print from preview

I've attached a patch with the adjustments needed for
ViewShell::PrintPreViewPage(..) 

OD->ayaniger:
Thx, I will review your provided patch for method
ViewShell::PrintPreViewPage(..) this week - taking over issue for review.

Are you fine with my adjustments of your revised patch?

OD->ayaniger:
I've reviewed your changes for the print method. In general the changes are ok,
but one scenario isn't considered. This scenario is a document containing pages
of different sizes, e.g. portrait and landscape pages. I've made the
corresponding adjustment to consider this scenario and also made a minor
simplification. Please have a look my revised patch, which I will attach soon.

When you are fine with my adjustments to your provided patch, I will integrate
it in cws swqbf105 for OOo 2.4

Created attachment 48219 [details]
print patch with adjustment and simplification

ayaniger->OD:
adjustments look good to me.

Ok. The patch provided by ayaniger and adjusted by OD will be integrated into
cws swqbf105

patch integrated into cws swqbf105 - changed files:
/sw/inc/pagepreviewlayout.hxx, 1.9.444.1
/sw/source/core/view/pagepreviewlayout.cxx, 1.11.424.1
/sw/source/core/view/viewpg.cxx, 1.28.144.1

OD->ayaniger:
Testing is everthing - I've already noticed again.
The following doesn't work with our patch:
- create a RTL document with a couple of pages
- switch to print preview
- choose a 4x3 print preview layout
- zoom in as far that only two columns of the print preview layout are visible
- select via mouse a visible page
- move the selection via arrow-keys to the left to an invisible page
--> invisible selected page isn't made visible

I think method <SwPagePreviewLayout::CalcStartValuesForSelectedPageMove(..)>
needs more adjustment.

OD->ayaniger:
More testing reveals, that after zooming in the print preview doesn't function
as excepted. Scrolling using the horizontal scrollbar doesn't work - not all
pages are shown.

ayaniger->od:

In 2.2.1, without the patch, I found that:

- when I select a 4x3 layout, print preview only shows two pages, even when I
zoom out, even when the doc is LTR. This doesn't seem right to me, and from your
comment I understand that with a 4x3 layout, you see more than two pages unless
you zoom in. Do I understand your comment correctly? 
 
- using the horizontal scrollbar doesn't work, even in LTR. This doesn't seem to
have been caused by the patch.

With the patch, I found that:

I was able to use the arrow-key to select an invisible page to the left. This is
different from what you describe, if I understand correctly. In the attached
document, print preview shows page 1 on the right, and page 2 on the left. I
selected page 2 with the mouse, moved the arrow key to the left, and I saw page
3. This seems to be ok.

Am I misunderstanding your description of the problems?

Created attachment 48330 [details]
Sample doc - text direction is RTL, both in Format/Page/Layout and default style

OD->ayaniger:
Hm, there seems to be a misunderstanding - probably I expressed myself not
correctly.
Let me try again:
(1) take OOo 2.x *without* the patch and open the document you recently attached.
(2) switch to print preview
(3) choose 4x3 print preview layout
--> all the 6 pages of the document are shown now in 4 columns and 3 rows.
Because you have only 6 pages in the document you have 4 pages in the first row
and 2 pages in the second row - the third and fourth column of the second row
are empty, also the third row is empty.
(4) zoom in (increasing the zoom factor) by using the "zoom in" button in the
print preview toolbar as long as only pages of the second column and the third
column are visible.
(5) now, you can use the horizontal scrollbar to bring the first column or the
fourth column into the view.
(6) select a certain visible page using a mouse click
(7) now, you can use the arrow keys to move the selection to other pages. If the
selection moves to an invisible page, this page comes into the view.
Step (5) and (7) works fine in OOo 2.x without our patch.

Now, open the document in a OOo version, which includes our patch, and repeat
the above steps.
As you will notice step (4) doesn't work as excepted. Also step (5) and (7)
doesn't work.

OD->ayaniger:
further investigation reveals, that everything works fine as long as all columns
are visible of a certain print preview layout. If not all columns are visible,
the calculation of the page positions and which pages are visible doesn't work
correct. Debug method <SwPagePreviewLayout::_CalcPreviewPages()> to see what
went wrong. It may also happen that a page, which should be visible isn't track
by method <SwPagePreviewLayout::_CalcPreviewPages()>. 
Please take back this patch issue for further adjustments.
In the meanwhile, I will undo the made changes in cws swqbf105.

ayaniger->OD:

OK, now I understand the problem. I was setting 4x3 in the Print Options dialog
instead of using the "Page Preview:Multiple Pages" button on the toolbar. 

Anyway, here's a patch for the preview, which I think fixes the bug. Sorry for
the delay, I've been away from OOo work for a while.

Created attachment 49160 [details]
Revised patch for preview code

Ok, I will review your new patch at the beginning of next week.

OD->ayaniger:
Your newest patch improved the last patch for a RTL layout of the Print Preview,
but we are not finished.
I observed the following defects:
(1) Open your given sample document from 2007-09-19. Switch to the Print
Preview. The default Print Preview layout is 2x1.
If the all columns fit into the window width, the pages are horizontal centered
anymore.
(2) Open your given sample document from 2007-09-19. Switch to the Print
Preview. Choose Print Preview layout 4x3 and zoom in as far as less than three
columns fit into the window width. Now use the horizontal scrollbar. You will
observe, that moving the scrollbar to the left will move the window content to
the right and vice versa.
Please have a look and adjust your revised patch accordingly.

OD->ayaniger:
Another note:
After method <SwPagePreviewLayout::_CalcPreviewPages()> has been performed, all
invisible pages aren't contained in container
<SwPagePreviewLayout::maPrevwPages> or its attribute <PrevwPage::bVisible> in
this container is FALSE.

Due to code freeze deadline for OOo 2.4, I set the target to OOo 3.0

OD->ayaniger:
Anything new for this patch issue?

OD->ayaniger:
Anything new for this patch issue?

Hi OD,
The Hebrew OOo project is currently in indefinate intermission pending further
funding from the Ministry of Finance or other source. We hope to be back later
in 2008. Best wishes, - yba

Thus, currently no progress on this issue

Adjusting target due to missed deadline.

Alan, as 3.1 is freezing this week, we have to move the target. I've taken 3.x,
please feel free to change it to anything you see fit.

Are you still working on that? Or should we set the type to "DEFECT"?

I haven't been working on this, and I won't be for at least a couple of months.

*** Issue 80802 has been marked as a duplicate of this issue. ***

OD->ayaniger:
Please consider also right-to-left vertical text documents (issue 80802), when
you continue your work on this issue. Thx.

I'm adding this comment to all open issues with Issue Type == PATCH.  We have 220 such issues, many of them quite old.  I apologize for that.  

We need your help in prioritizing which patches should be integrated into our next release, Apache OpenOffice 4.0.

If you have submitted a patch and think it is applicable for AOO 4.0, please respond with a comment to let us know.

On the other hand, if the patch is no longer relevant, please let us know that as well.

If you have any general questions or want to discuss this further, please send a note to our dev mailing list:  dev@openoffice.apache.org

Thanks!

-Rob

Reset assigne to the default "issues@openoffice.apache.org".