fix: Fix Nation Notes unable to share templates #7144
Conversation
- add prevent default to keep form from submitting on button - fix numerous php warnings - fix sql query error for orphan query - some restyle for colors
|
I still need to fix text template to allow template created in nation notes and vica versa. |
|
@juggernautsei I thought you were testing NN and not Text Notes. |
- adjust dialog - fix escape for attr from htmlspecialchar refactor
stephenwaite
changed the title
|
@juggernautsei This is testing very well for me. I'm glad you brought up sharing templates between NN and TN which I now allow. Would have done sooner if it was brought up. |
|
|
* Fix Nation Notes unable to share templates - add prevent default to keep form from submitting on button - fix numerous php warnings - fix sql query error for orphan query - some restyle for colors * - add allow share templaes between NN and Text Notes - adjust dialog - fix escape for attr from htmlspecialchar refactor * - fix component list to match theming (cherry picked from commit 9e7ce34)
|
fyi just cherry picked to v7.0.2 for next patch |
| echo "<textarea name='item' id='item' class='w-100'></textarea><br />"; | ||
| echo "<input type='button' name='save' value='" . htmlspecialchars(xl('Save'), ENT_QUOTES) . "' onclick='save_item()'><input type='button' name='cancel' value='" . htmlspecialchars(xl('Cancel'), ENT_QUOTES) . "' onclick=cancel_item('" . htmlspecialchars($row['cl_list_slno'], ENT_QUOTES) . "')></div></li>"; | ||
| echo "<textarea name='item' id='item' class='w-100 bg-dark text-light'></textarea><br />"; | ||
| echo "<input type='button' name='save' value='" . htmlspecialchars(xl('Save'), ENT_QUOTES) . "' onclick='save_item()'><input type='button' name='cancel' value='" . htmlspecialchars(xl('Cancel'), ENT_QUOTES) . "' onclick=cancel_item('" . attr_js($row['cl_list_slno'] ?? '') . "')></div></li>"; |
There was a problem hiding this comment.
should drop the outside ' around the attr_js call
(nice when i get to provide some escaping input every once i awhile to feel useful :) )
There was a problem hiding this comment.
it's fine Brady catch as catch can!:)
Maybe @stephenwaite can add this to his PR though I disagree it is needed. Again, inspect it in browser.
There was a problem hiding this comment.
@sjpadgett , Current codebase doesn't have the ' in all the calls and have not seen issue with this yet. Is there a use case where this breaks?
There was a problem hiding this comment.
Last issue I had was in one of my modules. I believe it was ringcentral but no matter. In this use case I suppose it doesn't matter but in the case where passing a string or an int was my issue I believe. Still either way it will parse and render the same up to the browser. I don't have an explanation for that!
I'll use it the way you intended though.
There was a problem hiding this comment.
Sounds good. Note in the special case of an int, attr_js will not add an ' (json_encode does not add surrounding quote to int), which would be a reason not to also hard-code quotes around this function (since would then convert an int to a string).
| <?php | ||
| if (AclMain::aclCheckCore('nationnotes', 'nn_configure')) { | ||
| ?> | ||
| <a href="add_template.php?list_id=<?php echo attr($_REQUEST['list_id']); ?>" onclick="top.restoreSession();" class="iframe_small btn btn-primary" title="<?php echo text(xl('Add Category')); ?>"><?php echo text(xl('Add Category')); ?></a> |
There was a problem hiding this comment.
attr instead of text for title attribute
| <?php | ||
| if (AclMain::aclCheckCore('nationnotes', 'nn_configure')) { | ||
| ?> | ||
| <a href="add_context.php" class="iframe_medium btn btn-primary" onclick="top.restoreSession();" title="<?php echo text(xl('Add Context')); ?>"><?php echo text(xl('Add Context')); ?></a> |
There was a problem hiding this comment.
attr instead of text for title attribute
| <?php | ||
| $where = ''; | ||
| if ($filter_context ?? null) { | ||
| $where .= " AND cl_list_id='" . $filter_context . "'"; |
There was a problem hiding this comment.
Really should be bound but this is why I hate touching these old scripts, everything needs to be done to them....
Again i'd appreciate if @stephenwaite would fix these or it will have to wait for my next PR
|
|
This picks up from PR #7130
Fixes #7129