View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0003514 | Composr alpha bug reports | [All Projects] General / Uncategorised | public | 2018-02-01 00:50 | 2018-02-12 00:02 |
Reporter | Chris Graham | Assigned To | Salman | ||
Severity | Feature-request | ||||
Status | resolved | Resolution | fixed | ||
Summary | 0003514: Tooltip escaping | ||||
Description | See screenshot. Ask if it's not clear. | ||||
Tags | No tags attached. | ||||
Sponsorship open | |||||
|
|
|
Alright. I don't think this issue was introduced by me. I spent 6+ hours tracking this down (the menu generation code is extremely complicated, its ->get_node() calls all the way down), it turns out these tooltips are stored in the 'translate' table and their serialized Tempcode in the 'text_parsed' columns has its HTML entities already escaped. So when they're referenced in Tempcode (i.e., {TEMPCODE*}), we get a double escaping issue. See 'SELECT t.text_parsed FROM cms.cms_cached_comcode_pages p LEFT JOIN cms.cms_translate t ON t.id=p.string_index WHERE id = 830;' I don't know what populates the translate table with these tooltips so I think you are better qualified to fix this way quickly or please guide me... |
|
Ok I'll take a look. The complexity is because I'm a stable genius. ;-) |
|
v10... page_grouping.php: $child_description = (is_object($link[4])) ? $link[4] : comcode_lang_string($link[4]); This is HTML encapsulated in Tempcode, and goes into the node structure, which ends up going to the menu branch template. In the menu branch template: onmouseover="if (typeof window.activate_tooltip!='undefined') activate_tooltip(this,event,'{TOOLTIP;^*}','auto');" We want them to be escaped here as we can't put HTML tags (deep) within an HTML escape happening. So apostrophes may be "double escaped". So all is okay so far. Actually we should really use '=' instead of '*' because we want to force escaping, even for language string output ('*' will not escape language strings, as they are assumed to already be HTML). We are taking something known to be HTML and deliberately escaping it. I'll fix that, but that's a side issue and invisible right now as we're not passing in language strings. v11... (page_grouping.php same) In the menu branch template: data-mouseover-activate-tooltip="['{TOOLTIP;^=}', 'auto']" The escape flow is... (initially) Banning IP addresses is useful to totally remove a user's ability to access the site; unfortunately, users can very easily switch IP addresses so it is not a perfect tool. --> (;) Banning IP addresses is useful to totally remove a user\'s ability to access the site; unfortunately, users can very easily switch IP addresses so it is not a perfect tool. --> (;^) Banning IP addresses is useful to totally remove a user\'s ability to access the site; unfortunately, users can very easily switch IP addresses so it is not a perfect tool. [i.e. no change] --> (;^=) Banning IP addresses is useful to totally remove a user\'s ability to access the site; unfortunately, users can very easily switch IP addresses so it is not a perfect tool. Note ";" will slash-escape HTML-escaped apostrophes. I'm not sure if that's needed anymore. It may have been an incorrect response to a bug years ago, and it is triggering the JSON5 parse errors in 0003513, because apparently JSON(5?) is less amiable to unnecessary escaping than normal JS. I'll remove this behaviour. "*"/"=" escaping should always happen after ";" escaping in HTML templates, as that's the logical pipeline. The browser will do these layer conversions... Stream layer: Banning IP addresses is useful to totally remove a user\'s ability to access the site; unfortunately, users can very easily switch IP addresses so it is not a perfect tool. DOM layer: Banning IP addresses is useful to totally remove a user\'s ability to access the site; unfortunately, users can very easily switch IP addresses so it is not a perfect tool. After my change though: Stream layer: Banning IP addresses is useful to totally remove a user's ability to access the site; unfortunately, users can very easily switch IP addresses so it is not a perfect tool. DOM layer: Banning IP addresses is useful to totally remove a user's ability to access the site; unfortunately, users can very easily switch IP addresses so it is not a perfect tool. Still none of this is anything to do with the apostrophe double escaping. That is coming from an unexpected place: title="{CAPTION*}{+START,IF_NON_EMPTY,{TOOLTIP}}: {$STRIP_TAGS,{TOOLTIP}} The 'title' attribute is turned into a tooltip automatically by the JS. This is racing against your own tooltip code. As can be seen in another bug I just found (but didn't add to the tracker), whereby hoving the link directly doesn't even show the tooltip. This should have been (my fault): title="{$STRIP_TAGS,{CAPTION}}{+START,IF_NON_EMPTY,{TOOLTIP}}: {$STRIP_TAGS,{TOOLTIP}} as the {CAPTION} was already "HTML-ready", just we need to not have the HTML-tags, leaving HTML entities in tact. This was not an issue in v10 because for whatever reason the racing wasn't happening or not causing an issue. Fixed in https://github.com/ocproducts/composr/commit/7135e556fcc3b9df2539441cd01a06f1ac401590 |
Date Modified | Username | Field | Change |
---|---|---|---|
2018-02-01 00:50 | Chris Graham | New Issue | |
2018-02-01 00:50 | Chris Graham | Status | non-assigned => assigned |
2018-02-01 00:50 | Chris Graham | Assigned To | => Salman |
2018-02-01 00:50 | Chris Graham | File Added: tooltip-escaping.png | |
2018-02-09 06:47 | Salman | Note Added: 0005481 | |
2018-02-09 06:58 | Salman | Note Edited: 0005481 | View Revisions |
2018-02-09 13:29 | Chris Graham | Note Added: 0005482 | |
2018-02-12 00:00 | Chris Graham | Note Added: 0005500 | |
2018-02-12 00:02 | Chris Graham | Status | assigned => resolved |
2018-02-12 00:02 | Chris Graham | Resolution | open => fixed |
2018-02-12 00:09 | Chris Graham | Note Edited: 0005500 | View Revisions |
2023-02-26 18:29 | Chris Graham | Category | General => General / Uncategorised |