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 for broken revprop processing #2749

Merged
merged 2 commits into from Jul 13, 2017

Conversation

Projects
None yet
2 participants
@robander
Member

robander commented Jul 13, 2017

Description

If I have a revision that is set to flag but does not specify an image, it results in this failure from the flag module:

[preprocess_flag] Failed to transform document: A value must be supplied for the
 parameter because the default value is not a valid instance of the required type

This occurs because when the flag module was updated to add typing to parameters and variables, two parameters for default-rev-start and default-rev-end were set to xs:string, but our code for handling default revisions calls those templates without any parameters (the 2 with-param elements are actually there but commented out).

Motivation and Context

The default start/end templates are there so that a process can add default start/end formatting (we've typically added a start-rev and end-rev graphic). By default they do nothing. Because the default parameters do not match the expected parameters, the whole module fails, and no flagging is added.

How Has This Been Tested?

This issue highlighted that we do not actually have any integration tests that use revision flags, so I've updated an existing test for <prop> flags so that it also tests <revprop> flags. The test now includes full default (flagged but no action), flag with color (no image) and flag with specified image.

Type of Changes

In addition to the updated test case, this fix changes 4 instances of xs:string so that they are now xs:string?.

An alternate fix would be to un-comment the sections that pass $lang and $bidi. I'm not sure which fix is better, or if it matters. Those have been commented out for a long time, so not knowing the reason, I left them commented out.

The fix is intended for hotfix/2.5.2 but opened against master because that branch doesn't yet exist.

robander added some commits Jul 13, 2017

Add test that fails with empty revprop
Signed-off-by: Robert D Anderson <robander@us.ibm.com>
Add fix for empty revprop
Signed-off-by: Robert D Anderson <robander@us.ibm.com>

@robander robander added the preprocess label Jul 13, 2017

@robander robander self-assigned this Jul 13, 2017

@jelovirt jelovirt added the bug label Jul 13, 2017

@jelovirt jelovirt changed the base branch from master to hotfix/2.5.2 Jul 13, 2017

@jelovirt

This comment has been minimized.

Show comment
Hide comment
@jelovirt

jelovirt Jul 13, 2017

Member

Changed the base to hotfix/2.5.2

Member

jelovirt commented Jul 13, 2017

Changed the base to hotfix/2.5.2

@jelovirt jelovirt merged commit 9718be8 into dita-ot:hotfix/2.5.2 Jul 13, 2017

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@jelovirt jelovirt added this to the 2.5.2 milestone Jul 13, 2017

@robander robander deleted the robander:hotfix/defaultRevprop branch Jul 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment