Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for #520 #521

Merged
merged 1 commit into from
Aug 26, 2021
Merged

Fix for #520 #521

merged 1 commit into from
Aug 26, 2021

Conversation

HelenaSabel
Copy link
Member

Syd’s fix for #520: it adds processing of @xml:id to the application of templates from <biblStruct> or <bibl> child of <listBibl>.

Just add processing of xml:id= to the application of templates from <biblStruct> or <bibl> child of <listBibl>.
@sydb sydb added the priority: releaseBlocker All tickets with this priority must be cleared in advance of the freeze before a release. label Aug 22, 2021
@sydb sydb added this to the Release 7.52.0 milestone Aug 22, 2021
@sydb sydb linked an issue Aug 22, 2021 that may be closed by this pull request
@sydb
Copy link
Member

sydb commented Aug 22, 2021

IMHO:

  • This PR must be reviewed by someone other than @sydb and @HelenaSabel, and must be merged prior to release.
  • Once a reviewer says go ahead, I think anybody can merge it in pronto, it does not require both reviewers to OK.

Copy link
Contributor

@martindholmes martindholmes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks OK to me, but I'm struggling to figure out what the original $id variable was supposed to be doing:

<xsl:variable name="id" select="@xml:id"/>

As far as I can see, it was never used, so something was already broken here. Do you see any use for it?

@martinascholger
Copy link
Member

I can confirm that the links from examples to BIB work in my local build. But there is no tooltip anymore (maybe only in my local version?).

@sydb
Copy link
Member

sydb commented Aug 22, 2021

Futhermucker. Do you think lack of tool-tips is just high priority, or outright release blocking?

@HelenaSabel
Copy link
Member Author

Same thing happens in my local installation as well, @martinascholger .
I took a quick look at @martindholmes JavaScript and I don’t see the problem: the parameter that it is being sent to the function showPopupBibl() matches an @id in BIBL.html. I don’t know why it defaults to linking to the bibliography (without showing the popup). I will take a closer look tonight.

@martindholmes
Copy link
Contributor

This is normal browser behaviour. AJAX won't run on you local machine, only when the page is coming from a server. It's not broken.:)

@sydb
Copy link
Member

sydb commented Aug 23, 2021

… I'm struggling to figure out what the original $id variable was supposed to be doing:
<xsl:variable name="id" select="@xml:id"/>
As far as I can see, it was never used, so something was already broken here. Do you see any use for it?

I saw so little use for it, I deleted it. 😁

@peterstadler
Copy link
Member

Thanks to @sydb and @HelenaSabel trying to explain the issue and the fix during todays Stylesheets meeting! It involves importing this Stylesheet into the Guidelines' build (https://github.com/TEIC/TEI/blob/59a2becc4a35f4a50a2518c605e1adef310e9f2c/P5/Utilities/guidelines.xsl.model#L20-L21) and some more intricacies. @sydb put it so: "We had to concede, in the end, that we are not entirely sure how this ever worked before these changes"

@peterstadler peterstadler merged commit 0e6740e into dev Aug 26, 2021
@hcayless hcayless deleted the iss516 branch September 11, 2022 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: releaseBlocker All tickets with this priority must be cleared in advance of the freeze before a release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

links from examples to BIB failing
5 participants