Bug 39118 - [PATCH] Handling of page-number-citation-last
Summary: [PATCH] Handling of page-number-citation-last
Status: NEW
Alias: None
Product: Fop - Now in Jira
Classification: Unclassified
Component: page-master/layout (show other bugs)
Version: trunk
Hardware: Other other
: P2 normal
Target Milestone: ---
Assignee: fop-dev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-03-27 16:53 UTC by Pierre-Henri Kraus
Modified: 2012-04-11 06:17 UTC (History)
0 users



Attachments
The patch file (19.05 KB, patch)
2006-03-27 16:54 UTC, Pierre-Henri Kraus
Details | Diff
Basic testcase to check the page-number-citation-last element (5.86 KB, text/xml)
2006-03-30 11:39 UTC, Pierre-Henri Kraus
Details
This file is the *complete* patch to handle page-number-citation-last in the different FO elements requested (49.45 KB, patch)
2006-04-15 12:30 UTC, Pierre-Henri Kraus
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pierre-Henri Kraus 2006-03-27 16:53:47 UTC
This patch add the handling of the page-number-citation-last element defined in
the XSL 1.1 spec
Comment 1 Pierre-Henri Kraus 2006-03-27 16:54:27 UTC
Created attachment 17995 [details]
The patch file
Comment 2 Pierre-Henri Kraus 2006-03-30 11:39:06 UTC
Created attachment 18004 [details]
Basic testcase to check the page-number-citation-last element

OK, sorry for the delay but JUnit was acting strange :p
2 questions:
- This test uses the "linefeed-treatment=preserve" property, is it ok ?
- Do you want more test for the element ?

And to Jeremias, i'll try to fax the ICLA today or tommorow (asking someone as
i don't have a fax myself), and if i can't i'll send it by mail...
Comment 3 Jeremias Maerki 2006-03-30 19:05:00 UTC
(In reply to comment #2)
> Created an attachment (id=18004) [edit]
> Basic testcase to check the page-number-citation-last element
> 
> OK, sorry for the delay but JUnit was acting strange :p
> 2 questions:
> - This test uses the "linefeed-treatment=preserve" property, is it ok ?

Yes, but I don't see what its function is in this particular test case.

> - Do you want more test for the element ?

If you don't mind, I have an additional wish: Testing page-number-citation-last
referencing an id on the page-sequence because that will be the major use case
for it. The most interesting part is then to combine that with a
page-position="last" where the last page is a blank page (see
page-position_last_1.xml).

> And to Jeremias, i'll try to fax the ICLA today or tommorow (asking someone as
> i don't have a fax myself), and if i can't i'll send it by mail...

Thank you! If possible, please try fax because we've had bad experiences with
normal mail.
Comment 4 Pierre-Henri Kraus 2006-04-15 12:30:05 UTC
Created attachment 18109 [details]
This file is the *complete* patch to handle page-number-citation-last in the different FO elements requested

Ignore the precedent patch, this one applies the same modifications + new ones
		
		Patch Explanation
		-----------------
		
OK, this is a file to help you understand the changes applied
to the source code to handle the page-number-citation-last
element. 

New Data Structures
-------------------

2 java.util.HashSet in AreaTreeHandler.java :

    private Set unresolvedLayoutManagers = new HashSet();
    private Set alreadyResolvedIDs = new HashSet();

The role of unresolvedLayoutManagers is to keep a list of 
"which formatting objects contributes IDs" as you proposed it.

The role of alreadyResolvedIDs is to keep a list of IDs already
resolved. That role might seems unnecessary, so here's a little 
example that i came across while doing the modification :

...
  <fo:block>page: <fo:page-number/>, bof2 is on page <fo:page-number-citation
ref-id="bof2"/></fo:block>
    <fo:block linefeed-treatment="preserve" id="bof3">page: <fo:page-number/>
      test
      test
      test
      test
      test
      test
    </fo:block>
    <fo:block break-before="page">page: <fo:page-number/></fo:block>
    <fo:block>page: <fo:page-number/></fo:block>
    <fo:block>page: <fo:page-number/> of <fo:page-number-citation
ref-id="eof1"/></fo:block>
    <fo:block>page(-last): <fo:page-number/> of <fo:page-number-citation-last
ref-id="bof3"/></fo:block>
...

Withouth the second Set, the problem that we come across is that the
layout of the block with the id 'bof3' is finished before the creation
of the page-number-citation-last layout manager. Due to the construction
of the PNCL-LM, the first step it does is to add the UnresolvedPageNumber
in the unresolvedLayoutManager list (there is no way to known in advance
if the page number can be resolved or not at this point). Each layout manager
is responsible of removing its ID from the unresolvedLayoutManager's list
and, at the same time to call for the resolution of the IDs its possess.
But as the ID of the block of the block layout manager is inexisting in the
list as the PNCL-LM haven't addded yet, it is not resolved when "it should".
The solution that i chose is to add each ID resolved "in advance" in a Set,
that the PNCL-LM will check to see if it can skip the "wait for resolve"
step, so it can immediately be resolved. 

I hope that this little explanation helped you to understand the changes.

What have been done -> handling of PageSequence ids and Block ids + Block level

ids (as page-number-citation)

Known limits -> no handling of IDs contained in a root, flow, and the table
related ids : table-header, table-footer, table-body ...
And, i don't know if it's a problem, but when the actual UnresolvedPageNumber
object is resolved, the font associated to it is null (this is due to the
"transient" state of the object i guess...) and the IPD can't be updated (only
the case with PageSequences).

New Code
--------
As explained above.
The source code changes are avail in the patch file.
Comment 5 Jeremias Maerki 2006-04-23 09:41:57 UTC
Pierre-Henri, I've applied your patch with modifications. See:
http://svn.apache.org/viewcvs?rev=396243&view=rev

Thanks a lot for that and sorry for the delay.

I found a remaining problem with your patch. When a formatting object spans
multiple pages, forward references are not properly resolved. You can see that
in the *_basic.xml test case which I've modified and disabled. It looks like you
didn't entirely get my hint last time. The problem is that addAreas() can be
called multiple times. An example: If a block spans multiple pages, it is called
once for each page it generates area for. Since you notify the AreaTreeHandler
after each call to addAreas (and not only after the last) a forward reference
gets the wrong page number (i.e. the one of the first area). Determining the
last call to addAreas for a formatting objects might be a little tricky, however.

Furthermore, you've only addressed block-level formatting objects and
page-sequence for ID processing, but some of the inline-level formatting objects
can similarly span multiple pages (like fo:inline for example). I don't think
this is critical since you might rarely refer to an inline-level ID with
page-number-citation-last. The most important use case is certainly referring to
an ID from page-sequence to achieve reliable "page x of y" without the "empty
block with ID" hack we've used until today.

I'm leaving the issue open for now.
Comment 6 Andreas L. Delmelle 2008-05-14 04:02:43 UTC
Small update on this one:
page-number-citation-last is currently correctly handled for fo:block, fo:inline and fo:list-block

Does not work yet for:
-> table-elements: ids not processed for table-header, table-footer, table-body and table-row; notifyEndOfLayout() is still called multiple times for table and table-cell.
-> block-containers: AbstractLM.isLast(Position) does not work reliably in this case, as the BlockContainerLM also wraps the auxiliary positions for its children's borders. Result is that the 'last' position in the iterator that the block-container receives during addAreas() is not one of its own. It is given a position index, though, which is greater than that of the last content element, so...
-> page-sequence: to be investigated
Comment 7 Glenn Adams 2012-04-07 01:41:39 UTC
resetting P2 open bugs to P3 pending further review
Comment 8 Glenn Adams 2012-04-11 03:20:44 UTC
increase priority for bugs with a patch
Comment 9 Glenn Adams 2012-04-11 06:17:39 UTC
change status from ASSIGNED to NEW for consistency