cwrc / DEPRECATED-CWRC-Writer

The Canadian Writing Research Collaboratory (CWRC) is developing an in-browser text markup editor (CWRCWriter) for use by collaborative scholarly editing projects.
http://www.cwrc.ca/projects/infrastructure-projects/technical-projects/cwrc-writer/
GNU General Public License v2.0
24 stars 17 forks source link

Note popup not working across schemas; embedded CWRC-Writer #420

Closed SusanBrown closed 6 years ago

SusanBrown commented 8 years ago

In both a TEI doc and the Orlando biography schema I cannot create a note. No tags appear in the structure panel on the left and the "NOTE:" label in grey that appears in the standalone version does not appear either.

With the orlando biography schema I get the following error message. I seem to be able to validate so I assume the problem is not that the app cannot access the schema.

image

ajmacdonald commented 8 years ago

Needs testing once embedded version is updated.

jefferya commented 8 years ago

Updated the embedded version on cwrc-dev-06. Received the following error when creating a note for a TEI doc (also in attache screenshot).

require.js?obno1y:8 Uncaught Error: Script error for: schema/undefined/mappings http://requirejs.org/docs/errors.html#scripterror

require.js?obno1y:34 GET http://cwrc-dev-06.srv.ualberta.ca/sites/default/libraries/CWRC-Writer/src/js/schema/undefined/mappings.js

untitled

ajmacdonald commented 8 years ago

The issue here is that the schema is unrecognized.

When note_tei.xml is being processed, the schema URL is determined http://cwrc.ca/schemas/cwrc_tei_lite.rng and then matched against the schemas loaded into writer.schemaManager.schemas:

http://cwrc-dev-06.srv.ualberta.ca/islandora/object/cwrc%3A2117047d-dd53-4d51-a271-1e8735f46819/datastream/SCHEMA/view
http://cwrc-dev-06.srv.ualberta.ca/islandora/object/cwrc%3AeventsSchema/datastream/SCHEMA/view
http://cwrc-dev-06.srv.ualberta.ca/islandora/object/cwrc%3AbiographySchema/datastream/SCHEMA/view
http://cwrc-dev-06.srv.ualberta.ca/islandora/object/cwrc%3AwritingSchema/datastream/SCHEMA/view
http://cwrc.ca/schemas/orlando_writing_v2.rng
http://cwrc.ca/schemas/orlando_biography_v2.rng
http://cwrc.ca/schemas/orlando_biography.rng
http://cwrc.ca/schemas/orlando_writing.rng
http://cwrc-dev-06.srv.ualberta.ca/islandora/object/cwrc%3AentrySchema/datastream/SCHEMA/view
http://cwrc.ca/schemas/cwrc_entry.rng

If the URL doesn't match any of the URLs in writer.schemaManager.schemas then it's assumed to be a custom schema. This then messes up the attempt to load the appropriate mappings.

SusanBrown commented 8 years ago

If it is a TEI document, then why would the schema not be recognized? @jefferya @ajmacdonald do either of you know why this would be the case?

ajmacdonald commented 8 years ago

The schema URLs don't match: http://cwrc.ca/schemas/cwrc_tei_lite.rng vs http://cwrc-dev-06.srv.ualberta.ca/islandora/object/cwrc%3A2117047d-dd53-4d51-a271-1e8735f46819/datastream/SCHEMA/view

The schema URL is used to determine the matching entry from here: https://github.com/cwrc/CWRC-Writer/blob/development/src/js/writerConfig.js

SusanBrown commented 8 years ago

@jefferya can you see how to fix this or is this a structural issue e.g. to do with a bigger issue such as this? https://github.com/cwrc/CWRC-Writer/issues/386

jefferya commented 8 years ago

For my example, the validation service and settings box appeared to pickup the proper schema. I'll do some digging in the CWRC-Writer integration module looking for why the validation picks up on the schema but not the note.

On Aug 17, 2016 10:22 AM, "SusanBrown" notifications@github.com wrote:

@jefferya https://github.com/jefferya can you see how to fix this or is this a structural issue e.g. to do with a bigger issue such as this? #386 https://github.com/cwrc/CWRC-Writer/issues/386

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cwrc/CWRC-Writer/issues/420#issuecomment-240466159, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1sqX1IT4FLZPQHPRwm2XfIefAiIYJoks5qgzVWgaJpZM4JZDzV .

ajmacdonald commented 8 years ago

The proper schema is being picked up for the base document because the schemaId override parameter is specified:

writer.fileManager.loadInitialDocument(window.location.hash, config.schemaId);

(from http://cwrc-dev-06.srv.ualberta.ca/sites/default/modules/islandora_cwrc_writer/js/islandora_cwrc_writer.js )

This avoids CWRC-Writer attempting to determine the schemaId when processing the document: https://github.com/cwrc/CWRC-Writer/blob/development/src/js/converter.js#L550

When loading the note template document, no schemaId was provided.

jefferya commented 8 years ago

My progress to date:

Embedded version works with TEI docs because when the Note layer opens, it tries to load a template based on the schemaManager.schemaId. In the case of TEI, the filename of the template is hard coded (i.e., https://github.com/cwrc/CWRC-Writer/blob/development/src/js/schema/tei/dialogs/note.js#L75).

The standalone version, there are multiple note.js files, one for each of orlando, TEI, and cwrcEntry as per https://github.com/cwrc/CWRC-Writer/blob/development/src/js/writerConfig.js.

The embedded version uses different schemaIds, onces based on the PID of the schema object. The codebase is trying to map the pid of the schema object to the template and the name matching fails (e.g., https://github.com/cwrc/CWRC-Writer/blob/development/src/js/schema/cwrcEntry/xml/note_cwrcEntry.xml)

One thing to note, the embedded version is referencing the correct directory and note.js (i.e., distinguishing between tei, orlando, and cwrcEntry). However, it's not passing this information along to note.js postSetup function. (https://github.com/cwrc/CWRC-Writer/blob/development/src/js/schema/cwrcEntry/dialogs/note.js#L86 and https://github.com/cwrc/CWRC-Writer/blob/development/src/js/schema/orlando/dialogs/note.js#L86)

Next steps, determine how the directory to note.js is determined and then how to override in the embedded version.

ajmacdonald commented 8 years ago

@jefferya if there's any assistance you need in figuring out the CWRC-Writer internals, let me know

jefferya commented 8 years ago

@ajmacdonald

Thank you Andrew for the offer. If you have a moment and it's easy to answer off the top of your head.

Where is the logic in the CWRC-Writer standalone that determines which of the three note.js (orlando, tei or cwrcEntry directories) files to use? I'm assuming it's related to the schemaId.

thank you.

ajmacdonald commented 8 years ago

It occurs in the dialogManager, which is listening for the schemaLoaded event: https://github.com/cwrc/CWRC-Writer/blob/development/src/js/dialogManager.js#L123

jefferya commented 8 years ago

The solution for the embedded version of CWRC-Writer and integration module involves constructing "note" template id from the schema id and loading the template id within the note.js postSetup function (e.g., note_biography.xml, note_writing.xml, note_event.xml, note_cwrcEntry.xml). Details here:

https://github.com/cwrc/CWRC-Writer/blob/development/src/js/schema/orlando/dialogs/note.js#L85-L87

The supported values for the schema ids are defined in the standalone CWRC-Writer default schema configuration file, namely tei, cwrcEntry, biography, writing, event (the latter three are Orlando) - these 5 are treated as the schema IDs within the Standalone CWRC-Writer and used to determine the note template. The embedded uses PIDs as schema id and thus doesn't populate the note template id with the supported values mentioned above thus causing the error.

https://github.com/cwrc/CWRC-Writer/blob/development/src/js/writerConfig.js

How to properly construct the template ids within the embedded CWRC-Writer without making life harder for issues like the follow: https://github.com/cwrc/CWRC-Writer/issues/215

My first thought, please offer critique, add another field to the schema definitions in writerConfig.js (the embedded version equivalent) and change note.js to conditionally use this field (https://github.com/cwrc/CWRC-Writer/blob/development/src/js/schema/orlando/dialogs/note.js#L85-L87)

The Drupal integration module would then be altered to allow setting this field. Maybe this area: https://github.com/jefferya/islandora_cwrc_writer/blob/7.x/includes/utilities.inc#L586-L672

ajmacdonald commented 8 years ago

@jefferya your solution makes sense. It keeps things within the directory for the schema, instead of relying on the schemaId information from writerConfig.js

ajmacdonald commented 8 years ago

@jefferya I just re-read your suggestion. Yes, the field would have to go in writerConfig.js (not the schema mappings file, like I was thinking, because it doesn't have access to the particular schema ID).

ajmacdonald commented 8 years ago

There's now an entityTemplates property: https://github.com/cwrc/CWRC-Writer/blob/development/src/js/writerConfig.js#L21 In use here: https://github.com/cwrc/CWRC-Writer/blob/development/src/js/schema/tei/dialogs/note.js#L75

jefferya commented 8 years ago

@SusanBrown

The "note" fix is now active on all the servers.

One side effect, when embedded CWRC-Writer first loads the RDF overlapping popup appears twice (due to following being called twice https://github.com/cwrc/CWRC-Writer/blob/master/src/js/converter.js#L701). I'll look into why and see if I can track down the reason. I may need to ping Andrew for help. The standalone is fine.

Also, I updated the stock schemas and templates in the embedded version (the ones with the human readable ids).

brundin commented 8 years ago

Great work on fixing the note issue, Jeff!

jefferya commented 8 years ago

@SusanBrown @ajmacdonald when you have a chance, would you be able to offer some advice on the javascript code used to initialize the embedded CWRC-Writer? The problem is the "XML and RDF (no overlap)" message box appears twice (because the function" writer.converter.doProcessing" is called twice at startup). I've tracked the calls back to their respective triggers and one javascript function specific to the embedded version used to initialize CWRC-Writer doesn't make sense to me - it seams unneeded (i.e., function in question loads a default doc into the CWRC-Writer and later another event replaces the default doc with the real doc). Andrew, are you able to offer any insights as to the necessity of the following function called when initiallizing CWRC-Writer (i.e., is such an approach needed for the stand-alone version)?

https://github.com/jefferya/islandora_cwrc_writer/blob/7.x/js/islandora_cwrc_writer.js#L258-L310

I'm trying to ask the question in the context of the stand-alone in the event Andrew is not familiar with the nuances of the embedded version.

The alternative, delete the function and test everything within the embedded CWRC-Writer.

ajmacdonald commented 8 years ago

@SusanBrown @jefferya My understanding of the embedded writer is that it's used to load/edit one document, and then closed. Is this correct?

The linked code seems to account for a situation where the user would change the schema through the settings dialog, which would require clearing of the current document. If there is another reason for this code, I can't imagine it.

If it's unlikely that the user will change the schema and create a new document, then the linked code seems unnecessary to me.

If it is likely, then a workaround would be to add listeners for the "loadingDocument" and "documentLoaded" events. You could then set a flag to determine if the "schemaLoaded" event was caused by a document load or by the user. If the document load is part of the initial document load, then you could avoid setting the document content twice (i.e. calling converter.doProcessing)

converter.doProcessing should really be private. The expected way to load a document is through converter.processDocument.

SusanBrown commented 8 years ago

I don't think that the user is at all likely to do what you suggest, Andrew. What you suggest makes sense to me.

There are two related problems that I'd like to raise here. Not sure how closely related they are but...

The first is one that I just flagged which is the unpopulated popup (screenshot below). This is appearing when the CWRC-Reader opens up, ie. in the View mode when it is completely irrelevant, even if it were populated, which it is not. Will solving the above problem also solve this one, by any chance?

The other is the pop-up appearing when the mini-CWRC-Writer starts up to edit a note. We really don't want this to happen. Can the system not assume that the mini-CWRC-Writer adopts the same settings as the larger one (as indeed it perhaps must?) and forego this popup entirely?

image

SusanBrown commented 7 years ago

Think this ok now. @colinraybrian pls confirm