Repository navigation
Fix AtomPub media naming, paging and XML output, and editor menus - #213
Merged
Merged
Conversation
Found while running a 7.0.0 snapshot on rollerweblogger.org. - Media uploads: derive the file extension from the media type without parameters or a +xml/+json suffix (image/svg+xml gives .svg), and keep a Slug's own extension when it matches the type. - Media collections: put the slash before the offset in the feed id and the next/previous links of named directories. - AtomWriter: leave out characters that XML 1.0 does not allow, in text and attribute values. - Media edit-media URIs: look the file up by directory and name; uploads no longer store the directory name as the file's original path. - Entry editor: upgrade Summernote to 0.9.1, which writes its menu icons as <i></i>; 0.8.12 wrote <i/>, which jQuery 3.5+ leaves open so it hides the menu labels. Limit the panel chevron in roller.css to collapse toggles, so it no longer lands on the editor's menu items.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bugs found while running a 7.0.0-SNAPSHOT build (Rollarcus deploy
dcd21bf35, which contains master359c4804c) on rollerweblogger.org with the new AtomPub server. Each fix has a test;CHANGES.mdhas a 7.0.0 bug-fix entry.mvn clean installpasses on JDK 17 (499 tests, 0 failures). Every item was also checked over HTTP against a local Roller built from this branch (Jetty, embedded Derby), not the live site.1. Media upload: file extension from the content type
MediaCollection.fileExtension()drops parameters (; charset=UTF-8) and a structured-syntax suffix (+xml,+json), and maps common types to their usual extension (image/svg+xml→svg,image/jpeg→jpg,text/plain→txt,application/ld+json→jsonld).withExtension()keeps a Slug's own extension when it matches the type (x.svg,photo.jpeg).Content-Type: image/svg+xmlwithSlug: roller-atompub-modelcreatesroller-atompub-model.svg;Slug: x.svgstaysx.svg;text/plain; charset=UTF-8givesnotes.txt..svgis stored asimage/svg+xml(from the container's MIME map) and, being an active type, is served asapplication/octet-streamwithContent-Disposition: attachment; filename="roller-atompub-model.svg"andX-Content-Type-Options: nosniff, at both the edit-media URI and/mediaresource/. That policy is unchanged.MediaCollectionTest.2. Media collection paging links
The feed id and the
next/previouslinks are built by one helper:/{h}/resources/{offset}for the default directory,/{h}/resources/{dir}/{offset}for a named one.pathhas no trailing slash, andgetCollection()reads the last segment as the offset, so both forms round-trip.diagrams;nextis.../resources/diagrams/20, answers 200 with the last 2 files and apreviouslink to/0.RollerAtomProtocolTest.testMediaCollectionNextLinksLeadToTheNextPagefollowsnextto page 2 for a named and the default directory. The guide already documents/{h}/resources/{dir}/{offset};docs/atompub/examplesare unaffected (AtomPubDocExamplesTestpasses).3. Characters that XML 1.0 does not allow
AtomWriter.xml()drops every character outside#x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF], including unpaired surrogates, and everywriteCharacters/writeAttributecall goes through it.GET /commentsreturns a feed that parses, with the contentbeforeafterend.AtomWriterTest.testCharactersIllegalInXmlAreDropped(comment content with\u0014, entry title with\u0000and a lone surrogate, attribute value with\u001F; the feed parses back) andtestXmlFilterKeepsLegalText.4. Entry editor: line-height menu shows no values
Reproduced on master. Two causes:
<i class="note-icon-menu-check"/>. HTML has no self-closing<i>; jQuery before 3.5 expanded it to<i></i>, jQuery 3.5+ does not, so the browser keeps the<i>open and the label ends up inside it:<i class="note-icon-menu-check"> 1.0</i>. Summernote hides that<i>(.note-check li a i { visibility: hidden }), so the label disappears. jQuery is 3.7.1 on master and was in 6.1.6, so this is not new in 7.0.roller.cssputs a Glyphicons chevron after every.panel-heading a(for the entry editor's collapsible panels). Summernote's Bootstrap 3 toolbar is a.panel-heading, so every menu item got a chevron. That is the stack of glyphs.Fix: upgrade Summernote to 0.9.1 (
org.webjars.npm), which writes<i ...></i>and also fixes 0.8.12's line-height check never being set; and limit the chevron rule toa[data-toggle="collapse"]. Checked in the local editor: labels 1.0–3.0 show without chevrons, choosing 1.5 setsstyle="line-height: 1.5;", a draft saves and reloads it, paste runs, no console errors.EntryEditorAssetsTestchecks that the Summernote script loaded byhead.jspis on the classpath and does not build<i .../>icons, and that every.panel-heading a…:afterrule is limited to collapse toggles.5. Custom-theme weblogs with templates but no renditions (investigation only)
Not fixed here. Findings:
500-to-510script createscustom_template_renditionand copiesweblog_custom_template.templateinto it as STANDARD renditions, but only runs when the database version is below 510.rol_templatecodeand copied nothing; it was renamedroller_templatecode(ROL-2000, 2014-05) and thencustom_template_rendition(ROL-2039, 2014-07, first released in 5.1.2).rol_templatecodetable matches this.PageServletthen finds no renderer and answers 404. A fallback would have no template text to fall back to for a custom theme, so the data has to be repaired. Released upgrade paths are not affected, so I did not add repair code to the installer.Suggested repair (check first: does
rol_templatecodehave rows for those template ids? doesweblog_custom_templatestill have atemplatecolumn?):If neither source has the text, switching those weblogs to a shared theme (and back to custom, if wanted) recreates the templates.
6. Also fixed: edit-media GET answered 404
Found while testing item 1.
getMediaResource()looked uploads up byoriginalPath, butpostMedia()stored only the directory name there, soGETon every edit-media URI answered 404. It now usesgetMediaFileByPath()(as PUT and DELETE do) and loads the content; uploads no longer setoriginalPath, which is for legacy upload URLs.JPAMediaFileManagerImpl.getMediaFileByPath()returns null instead of throwing for an unknown directory.RollerAtomProtocolTest.testUploadedMediaCanBeReadThroughItsEditMediaUri(named and default directory).