-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
Upgrade PHPWord #13686
Upgrade PHPWord #13686
Conversation
(Standard links)
|
CRM/Core/Menu.php
Outdated
$xml = simplexml_load_file($name); | ||
self::readXML($xml, $menu); | ||
libxml_disable_entity_loader(TRUE); |
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.
The general pattern for doing this safely as well-behaved member of a complex system is:
$backupValueThatIDontMakeAnyAssumptionsAbout = readTheGlobalValue():
setTheGlobalValue($theValueThatILike);
try {
doTheMainThing();
}
finally {
setTheGlobalValue($backupValueThatIDontMakeAnyAssumptionsAbout);
}
On a case-by-cases basis, there might be a prettier way (e.g. maybe depending on the contract of the specific global-thing or maybe using CRM_Utils_AutoClean::swap()
or maybe using CRM_Utils_GlobalStack
).
b3b29db
to
4947d38
Compare
@totten got it to pass :-) |
@seamuslee001 are there remaining reasons not to merge now? |
nothing on my side but would appreciate Tim's opinion as well |
Patch phpword install to fix global handling of disable libxml entity loader Fix phpoffice/common as well
4947d38
to
66a7c25
Compare
Jenkins re test this please |
We should actually go to the latest version I think My experience with phpword is that we have had more than one case where the fix to some reported issue is to upgrade - as long as it's compatible with our min php version I think keeping it current solves more problems than it's likely to risk causing |
Looks like the latest is v0.16. @seamuslee001 will that newer version still require your entity_loader patch? |
@colemanw yes it would still need those patches |
So I'm happy to merge this as a 'technical' fix to the problem of 'how do we upgrade phpword - although getting the latest bug fixes would be better |
No description provided.