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

Second instance of annotation mis-placed #122

Closed
GilShalit opened this issue Jul 12, 2022 · 7 comments · Fixed by #125
Closed

Second instance of annotation mis-placed #122

GilShalit opened this issue Jul 12, 2022 · 7 comments · Fixed by #125
Labels
bug Something isn't working

Comments

@GilShalit
Copy link

GilShalit commented Jul 12, 2022

At least two annotation types (Abbreviation and Regularization) have a problem in their implementation.

To reproduce, open a file in the annotation samples of the TEI-Publisher annotate samples collection, either locally or on the server. The steps below are for 3267.

  1. Highlight the word domino, select the Regularization annotation, enter 'dom' as the regularization value. save the annotation and then save changes. The annotation is fine.
  2. Delete this annotation and save changes.
  3. Highlight a word that comes before domino, e.g. 'Christo', select the Regularization annotation, enter 'chr' as the regularization value. save the annotation and then save changes. The annotation is fine.
  4. Highlight again the word domino, select the Regularization annotation, enter 'dom' as the regularization value. save the annotation and then save changes. The annotation is not OK - it is placed a little later in the line!

Some notes:

  1. After placing the second annotation (step 4), saving it but not all changes, the annotation is placed correctly in the main pane, but inspecting the TEI tab shows that the second annotation is already placed incorrectly. After saving changes and inspecting the XML, this is what is saved to file.
  2. The same behavior happens for the Abbreviation annotation. On the other hand, the Sic annotation works just fine, as do other annotations I tested. I could not find a difference between Abbreviation and Regularization on the one hand and sic on the other, in the way they are declared in annotate.html and annotation-config.xqm.
  3. Some custom annotations I created produce the wrong behavior of Abbreviation and Regularization.
@joewiz
Copy link
Member

joewiz commented Jul 13, 2022

I was able to reproduce @GilShalit's report for triggering the primary issue. Here's a screenshot showing how the annotation interface looks before and after selecting the "merge and save annotations to TEI" button (i.e., the "save button") in step 4 of his directions.

Before

Screen Shot 2022-07-13 at 11 24 05 AM

After

Screen Shot 2022-07-13 at 11 24 30 AM

@joewiz joewiz added the bug Something isn't working label Jul 13, 2022
@line-o
Copy link
Member

line-o commented Aug 11, 2022

It would be valuable to see if there are differences in the markup created.

@rowissmann
Copy link

There is a bug in modules/lib/api/annotations.xql
Change line 353 to:
let $primary := $node/tei:sic | $node/tei:abbr | $node/tei:orig

@GilShalit
Copy link
Author

Just wanted to confirm that @rowissmann's fix does solve the reported issue. I guess this can be closed but maybe only after making the change in the distribution package.

@joewiz
Copy link
Member

joewiz commented Aug 22, 2022

This is great news!

@rowissmann Could you confirm that you mean this line? https://github.com/eeditiones/tei-publisher-app/blob/master/modules/lib/api/annotations.xql#L370

@GilShalit
Copy link
Author

I am attaching the image @rowissmann put in the slack channel, with the highlighted line that needed replacing. It seems there have been some other changes to the module since the current distribution was finalized for v 7.1.

image

@line-o
Copy link
Member

line-o commented Aug 23, 2022

Looking at the current line (thanks @joewiz)

let $primary := $node/tei:sic | $node/tei:abbr

The fix is to add tei:orig to the union as suggested by @rowissmann

let $primary := $node/tei:sic | $node/tei:abbr | $node/tei:orig

I will open a PR if @rowissmann or someone else does not beat me ;)

The harder part will be to come up with a reliable test for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants