The Stylizer may use the same CSS ID for multiple panes/panels with the same palette settnigs:
function ctools_stylizer_get_css_id($plugin, $settings) {
return 'ctools-stylizer:' . $settings['name'] . ':' . md5(serialize($settings['palette']));
}
As a result, it may lead to issues saving Stylizer settings.
This may also cause https://www.drupal.org/node/2759869 .
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | ctools-fix_stylizer_css_id-2897741-1-D7.patch | 1.46 KB | abramm |
Comments
Comment #2
abrammPatch attached.
Comment #3
id.tornado commentedit works for me
Comment #4
geek-merlinOnly CSS IDs changed.
Comment #5
joelpittetHow does this "work for you"? And more importantly how does this manifest as a bug?
It's also changing a path and a name as well as the CSS id.
Comment #6
geek-merlin> how does this manifest as a bug?
IS:
> The Stylizer may use the same CSS ID for multiple panes/panels with the same palette settnigs:
This is evident from the cited code.
Duplicate CSS IDs are a bug in itself (invalid HTML, maybe SEO).
Code change is trivial and makes a simple change to hashing all settings.
Comment #7
geek-merlinUps
Comment #8
joelpittetOk so
ctools_css_clear($id)will fail to remove the old CSS. May need to clear outctools-stylizer:files in an update hook?Comment #9
geek-merlinNo prob, will be done on next CC via ctools_css_flush_caches().
Comment #10
joelpittetThat's a good point it clears
public://ctools/cssalthough doesn't clearpublic://ctools/style.ctools_stylizer_cleanup_style()Tries but uses the generated name's specific path to delete generated images.I appreciate you trying to ease my mind on this, I don't want to cause a unnecessary regression.
Comment #11
geek-merlinIt's good you're critical, maybe i've been not thorough enough.
Now i see that stylyzer not only builds CSS, but some preview image bundled in a dir. ctools_stylizer_get_settings_name() tells us its name is the stylyzer name (so no problem here) OR for temp styles "_temporary-[MD5]".
The biggest problem from that might be some rare leftover files with no regression risk. Preview icons will be rebuilt on style resave.
Would be fine for me, but i won't prevent perfectionists from implementing a hook_update ;-).
Comment #12
abrammSorry, I wasn't probably clear enough when I created the issue.
The problem with IDs is that the same style may apply to different content panes.
I.e having panes A and B both using Stylizer with the same palette settings but different font settings, the same font setting will apply to both.
As for an update hook, how about fetching all Stylizer files from ctools_css_cache? E.g.:
SELECT * FROM {ctools_css_cache} WHERE cid LIKE 'ctools-stylizer:%';We can then walk through all files in the update hook batch callback and call ctools_css_clear(). That will remove all generated Stylizer CSS files.
Comment #13
japerryDrupal 7 is no longer supported, closing.