-
Notifications
You must be signed in to change notification settings - Fork 25
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
replace proprietary exist:serialize options #418
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx for fixing the leftovers!
I think you’re completely right that the serialization options are not appropriate for the controller.xql
Hi @bwbohl , why are you adding all these merge commits (that will apparently be included in the history)? |
There’s a button – I click the button … sorry The controller serialisation question is clear from what you stated above. Please don't hesitate to remove the serialisation. |
Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
many thanks
leftovers from #414 spotted by @bwbohl
Refs #362
To discuss
The current output method of
add/controller.xql
("method=xhtml media-type=application/xhtml+html") seems incorrect to me. In fact, it returns a<dispatch>
element so it should be considered "method=xml media-type=application/xml". Looking at various eXist apps (eXide, Monex, etc.), those never state the output method explicitly, so I assume they are relying on the default "xml". Only the Dashboard app has something different, yet I consider this an error?https://github.com/eXist-db/dashboard/blob/35ca2c9d5efc6ac27b28a6bfdb4f499bb622a2d8/controller.xql#L5-L7
Nevertheless, I'm not 100% sure so I copied over the existing media type and output method and put it here for discussion.