TOC Internal Page Jumps (especially Roman) only work in some instances

[nrshapiro]

When checking the latest book TOC I noticed the internal (roman page) links don't work. I thought Gavant had those working so I tried several different books.

Some of the links here work: https://stage.pep-web.org/browse/document/ZBK.033.0000A?index=28&page=PR0005 and you can see the link was added by the client:

But the new book doesn't work. Nor do several others I tried like this one:

https://stage.pep-web.org/browse/document/ZBK.048.0000A

[nrshapiro]

See also #643

[jordanallen-dev]

@nrshapiro @ocappello

This seems to be an XML issue. When a user clicks on one of these links, the following code is triggered:

In the cases where the link doesn't work, the condition on line 477 is not met, meaning it can't find a matching data-page-start attribute within the DOM

The link is configured correctly, it's destination isn't

vs

The first image is taken from ZBK.033.0000A where the HTML contains the attribute / value and the link works

The second image is taken from ZBK.048.0000A where the HTML contains the attribute, but with no value and the link does not work

[jordanallen-dev]

@nrshapiro This is exactly what you identified in #643

[nrshapiro]

@jordanallen-dev @ocappello

Jordan, what you're looking at is not XML, it's HTML. And the HTML is created by the client app.

A relevant quote from this issue:

""" Some of the links here work: https://stage.pep-web.org/browse/document/ZBK.033.0000A?index=28&page=PR0005 and you can see the link was added by the client:"""

from the related issue you point to: https://github.com/Psychoanalytic-Electronic-Publishing/PEP-Web-User-Interface/issues/643

"""I don't know what they are using to trigger population of that attribute from the XML, but it may be "nextpgnm" which is in the page number element on the page before, just before the start of the new page. But both Glover and Money-Kyrle have these populated correctly, so it must be a client issue."""

[jordanallen-dev]

@jordanallen-dev @ocappello

Jordan, what you're looking at is not XML, it's HTML. And the HTML is created by the client app.

A relevant quote from this issue:

""" Some of the links here work: https://stage.pep-web.org/browse/document/ZBK.033.0000A?index=28&page=PR0005 and you can see the link was added by the client:"""

from the related issue you point to: #643

"""I don't know what they are using to trigger population of that attribute from the XML, but it may be "nextpgnm" which is in the page number element on the page before, just before the start of the new page. But both Glover and Money-Kyrle have these populated correctly, so it must be a client issue."""

I'm aware it's not the XML in the screenshots

I was assuming the HTML renderer was taking this attribute directly from the XML and that it was missing from the source as an initial search didn't show up any results in the renderer

I'll dig a little deeper and see what the client uses to generate it

[jordanallen-dev]

@nrshapiro

Found it. I did a text search for data-page-start= instead of data-page-start which is why it never showed up

Uses nextpgnm as you thought

[jordanallen-dev]

This was an XML consistency issue

When there is no preceding page break siblings, the renderer finds looks for the next page break _within the parent block__ - one level higher

In the one where the links work, the XML looks like this:

Notice how the <pb> is not inside the <unit> block. When the <h1> is rendered, it checks for the next <pb> one level up

If we look at the XML where the look does not work:

We can see it is inside the <unit> block.

Instead of having to introduce better consistency retroactively, I've implemented a fix that checks for the next <pb> both in the parent and current block and I'm currently deploying it to stage

cc @ocappello @nrshapiro

[jordanallen-dev]

@nrshapiro @ocappello

Now working as expected:

https://stage.pep-web.org/browse/document/ZBK.033.0000A https://stage.pep-web.org/browse/document/ZBK.048.0000A

[jordanallen-dev]

Continued in #643