Issue 80209 - Page Preview in RTL should order pages from Right-to-Left
Summary: Page Preview in RTL should order pages from Right-to-Left
Status: CONFIRMED
Alias: None
Product: Writer
Classification: Application
Component: formatting (show other issues)
Version: OOo 2.2
Hardware: All All
: P3 Trivial with 1 vote (vote)
Target Milestone: ---
Assignee: AOO issues mailing list
QA Contact:
URL:
Keywords: BIDI
: 80802 (view as issue list)
Depends on:
Blocks: 114236
  Show dependency tree
 
Reported: 2007-07-31 09:50 UTC by alan
Modified: 2017-05-20 11:15 UTC (History)
5 users (show)

See Also:
Issue Type: PATCH
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments
Reverses page order in Page Preview for RTL (1.17 KB, patch)
2007-07-31 09:51 UTC, alan
no flags Details | Diff
Revised patch (4.73 KB, patch)
2007-09-04 12:20 UTC, alan
no flags Details | Diff
revised patch with minor adjustments (5.88 KB, patch)
2007-09-10 11:30 UTC, Oliver-Rainer Wittmann
no flags Details | Diff
handles RTL in print from preview (2.80 KB, patch)
2007-09-10 16:08 UTC, alan
no flags Details | Diff
print patch with adjustment and simplification (3.40 KB, patch)
2007-09-13 14:09 UTC, Oliver-Rainer Wittmann
no flags Details | Diff
Sample doc - text direction is RTL, both in Format/Page/Layout and default style (17.63 KB, application/vnd.oasis.opendocument.text)
2007-09-19 15:01 UTC, alan
no flags Details
Revised patch for preview code (5.88 KB, patch)
2007-10-25 19:00 UTC, alan
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description alan 2007-07-31 09:50:36 UTC
When the Page Preview window displays multiple pages, it orders them from left
to right. For RTL languages, the pages should be ordered right-to-left. I'm
attaching a patch which does this.
Comment 1 alan 2007-07-31 09:51:56 UTC
Created attachment 47221 [details]
Reverses page order in Page Preview for RTL
Comment 2 michael.ruess 2007-07-31 10:49:43 UTC
MRU->HI: please sent this PATCH issue to responsible developer. Thanks!
Comment 3 h.ilter 2007-07-31 15:02:59 UTC
HI->OD: Please evaluate.
Comment 4 Oliver-Rainer Wittmann 2007-08-02 08:10:19 UTC
review of patch in progress.
Comment 5 Oliver-Rainer Wittmann 2007-08-02 11:03:30 UTC
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?
Comment 6 alan 2007-09-04 12:18:23 UTC
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.
Comment 7 alan 2007-09-04 12:20:17 UTC
Created attachment 47950 [details]
Revised patch
Comment 8 Oliver-Rainer Wittmann 2007-09-05 07:40:43 UTC
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.
Comment 9 Oliver-Rainer Wittmann 2007-09-10 11:28:50 UTC
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.
Comment 10 Oliver-Rainer Wittmann 2007-09-10 11:30:07 UTC
Created attachment 48105 [details]
revised patch with minor adjustments
Comment 11 Oliver-Rainer Wittmann 2007-09-10 11:32:49 UTC
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.
Comment 12 Oliver-Rainer Wittmann 2007-09-10 11:33:59 UTC
forgot to assign back to ayaniger ;-)
Comment 13 alan 2007-09-10 16:08:04 UTC
Created attachment 48137 [details]
handles RTL in print from preview
Comment 14 alan 2007-09-10 16:12:42 UTC
I've attached a patch with the adjustments needed for
ViewShell::PrintPreViewPage(..) 
Comment 15 Oliver-Rainer Wittmann 2007-09-11 10:39:54 UTC
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?
Comment 16 Oliver-Rainer Wittmann 2007-09-13 14:07:15 UTC
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
Comment 17 Oliver-Rainer Wittmann 2007-09-13 14:09:15 UTC
Created attachment 48219 [details]
print patch with adjustment and simplification
Comment 18 alan 2007-09-16 14:33:29 UTC
ayaniger->OD:
adjustments look good to me.
Comment 19 Oliver-Rainer Wittmann 2007-09-17 09:20:53 UTC
Ok. The patch provided by ayaniger and adjusted by OD will be integrated into
cws swqbf105
Comment 20 Oliver-Rainer Wittmann 2007-09-17 09:33:49 UTC
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
Comment 21 Oliver-Rainer Wittmann 2007-09-17 09:58:04 UTC
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.
Comment 22 Oliver-Rainer Wittmann 2007-09-17 10:51:55 UTC
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.
Comment 23 alan 2007-09-19 14:59:21 UTC
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?

Comment 24 alan 2007-09-19 15:01:46 UTC
Created attachment 48330 [details]
Sample doc - text direction is RTL, both in Format/Page/Layout and default style
Comment 25 Oliver-Rainer Wittmann 2007-09-25 09:34:43 UTC
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.
Comment 26 Oliver-Rainer Wittmann 2007-09-26 11:35:16 UTC
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.
Comment 27 alan 2007-10-25 18:58:18 UTC
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.
Comment 28 alan 2007-10-25 19:00:43 UTC
Created attachment 49160 [details]
Revised patch for preview code
Comment 29 Oliver-Rainer Wittmann 2007-10-26 12:44:39 UTC
Ok, I will review your new patch at the beginning of next week.
Comment 30 Oliver-Rainer Wittmann 2007-10-30 15:13:39 UTC
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.
Comment 31 Oliver-Rainer Wittmann 2007-10-30 15:18:46 UTC
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.
Comment 32 Oliver-Rainer Wittmann 2008-01-14 11:01:08 UTC
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?
Comment 33 Oliver-Rainer Wittmann 2008-02-18 08:55:50 UTC
OD->ayaniger:
Anything new for this patch issue?
Comment 34 yba 2008-02-19 11:15:42 UTC
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
Comment 35 Oliver-Rainer Wittmann 2008-03-26 09:33:34 UTC
Thus, currently no progress on this issue

Adjusting target due to missed deadline.
Comment 36 Mathias_Bauer 2009-01-27 13:28:34 UTC
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"?
Comment 37 alan 2009-01-27 13:32:17 UTC
I haven't been working on this, and I won't be for at least a couple of months.
Comment 38 Oliver-Rainer Wittmann 2009-04-09 08:34:22 UTC
*** Issue 80802 has been marked as a duplicate of this issue. ***
Comment 39 Oliver-Rainer Wittmann 2009-04-09 08:36:20 UTC
OD->ayaniger:
Please consider also right-to-left vertical text documents (issue 80802), when
you continue your work on this issue. Thx.
Comment 40 Rob Weir 2013-03-11 15:01:01 UTC
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
Comment 41 Marcus 2017-05-20 11:15:22 UTC
Reset assigne to the default "issues@openoffice.apache.org".