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 broken rendering of maps inside lists #1311

Merged
merged 10 commits into from
Nov 29, 2020

Conversation

tcnh
Copy link
Contributor

@tcnh tcnh commented Nov 26, 2020

Since a (long) while, putting one or more hashmaps inside a list caused the hashmap to be escaped, resulting in hard-to-read test result pages. This PR fixes that, specifically for lists, so that other (unconventional) html doesn't break the page's rendering.

Effect of these changes below in before (left) and after (right) situations:
image

Copy link
Collaborator

@fhoeben fhoeben left a comment

Choose a reason for hiding this comment

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

The current list formatter is very limited and displaying nested tables in one example of that.

BUT the list formatter in Slim is configurable, so one can configure lists to be displayed differently.
As an example of another formatter please look at https://github.com/fhoeben/hsac-fitnesse-fixtures/blob/master/src/main/java/nl/hsac/fitnesse/slim/converter/NumberedListConverter.java (which my fixtures allow to be activate via a call to ListFixture's display lists numbered
I enable this converter when I need to work with complex structures where maps and lists are nested. There's also a method to switch back to the default formatter, because I recall some Tables don't respond well to different list formatting and when checking simple lists the square braces format is quite nice.

The regex approach to finding the lists does not feel quite 'right'. But I believe the wiki does not have access to the current list formatter (as that lives in the test runner's process).

Does this change break the usage of custom list formatters?
Could you add some tests?

src/fitnesse/testsystems/slim/HtmlTable.java Outdated Show resolved Hide resolved
src/fitnesse/testsystems/slim/HtmlTable.java Outdated Show resolved Hide resolved
tcnh and others added 2 commits November 26, 2020 14:27
Co-authored-by: Fried Hoeben <github@hsac.nl>
Co-authored-by: Fried Hoeben <github@hsac.nl>
@tcnh
Copy link
Contributor Author

tcnh commented Nov 27, 2020

The list formatter is not the issue, that works fine. This change should not impact anything but the rendering of lists in slim tables. The mapconverter change is needed to make sure that maps containing lists that contain maps - that sounds pretty specific :-) - are not escaped by the map converter.

What breaks the formatting is the fact that slim's HtmlTable has a very strict rule to determine if a cell qualifies as HTML. It has to be 'HTML-only' (or a symbolassignment containing just html). This works for all situations I know, except lists containing HTML (practically: lists containing hashes).

I experimented with setting the 'qualifiesAsHtml-rule' less strict (as in .HTMLPATTERN. instead of ^HTMLPATTERN$), but that breaks wiki formatting in some cases where html-output is rendered in cells.

One could even argue that the not-escaping of HTML in all cases other than explicit conversions such as lists or maps is unwanted, as it mostly results in broken result pages.

I'll add some tests to show that only lists are affected

@fhoeben fhoeben merged commit a0c8f5f into unclebob:master Nov 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants