As of issue #13, the solution seems to be hashing all deltas as opposed to only those longer than 32 characters. In order to implement this, we need to do two things:
- Make the change illustrated in #3
- Write an update hook to update the effected deltas in the block table
We cannot sanitize the delta on input because the bad characters contribute to the uniqueness of the delta and also serve as separators for the different parts of the facet that the delta is associated with. We don't want to sanitize the data on output because there are only limited locations where the bad characters aren't filtered and we would have to write code targeting those specific areas. See #13 for more details.
Original Post
I am in the process of moving to FacetAPI, migrating my old to new blocks. I show my blocks through Context. One block, a multi-value textfield (list<text>
), got a strange block CSS ID,
<tr id="facetapi-search_api@flatstore:block:color"
Note the @ sign, which breaks JavaScript in the Context blocks UI.
Other FacetAPI blocks got a hashed ID, "facetapi-23132-Ls9Hv4CHzroOMWtunOOHom3uv", but this color facet block didn't,...
The same thing happens to a few other blocks,
facetapi-search_api@flatstore:block:size
I can't figure this out, what is causing this?
Comment | File | Size | Author |
---|---|---|---|
#15 | facetapi-1284156-15.patch | 2.45 KB | cpliakas |
#9 | bad-class-names.jpg | 431.19 KB | cpliakas |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedThe problem is really the block delta, for example:
/admin/structure/block/manage/facetapi/search_api@flatstore:block:size/configure
A delta "search_api@flatstore:block:size" doesn't seem right :)
The solution would be to force-hash all block IDs. Let's have a look how to do that.
Comment #2
Anonymous (not verified) CreditAttribution: Anonymous commentedIt's an issue with Facet API.
Summary: Facet API's integration with Search API generated block delta's like
search_api@flatstore:block:size
which breaks jQuery (can't have @ signs in CSS ID's!)Comment #3
Anonymous (not verified) CreditAttribution: Anonymous commentedThis seems to be the problem:
Delta's shorter than 32 chars are not hashed, but Search API's delta's have "@" signs in them. So they should always be hashed,... Solution could be:
What do you think? Should we force-hash all delta's, or is Search API providing bad delta's?
Comment #4
drunken monkeyapachesolr's Facet API searchers also have @s in them, so if this really the problem (and I agree it looks like it) than it's a Facet API problem, not a usage problem. In any case, even if implementors were responsible for providing valid JS identifiers as searcher IDs, this would have to be specified.
So yes, always hashing block deltas seems like the best option to me. It also looks more consistent that way. ;) But let's see what Chris says.
Comment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedYeah, you can't use @ in a jQuery selector,
$('block-id@search-api')
breaks JS. I noticed it in the Context UI blocks system.Comment #6
cpliakas CreditAttribution: cpliakas commentedHmm, I 100% agree that these characters shouldn't be allowed. I thought we fixed that pre-beta1 (guess not!). I will try to confirm this bug and then see why these characters are making their way into the markup.
Thanks for reporting,
Chris
Comment #7
cpliakas CreditAttribution: cpliakas commentedSo after some research, this issue is not present in Apache Solr Search Integration, and Facet API appears to use the drupal_html_id() and drupal_html_css() functions appropriately. There was also an issue closed some time ago at #1150662: Facet blocks not remembering overridden settings which could be related to what is posted here. The block deltas are escaped properly by core, so if you are using the Context module which the title suggests, I would suspect that Context isn't properly escaping the IDs.
Comment #8
Anonymous (not verified) CreditAttribution: Anonymous commentedThe id's show in the Drupal core block list, and in their URLs, like
/admin/structure/block/manage/facetapi/search_api@flatstore:block:size/configure
Is that normal? If it is, then indeed it's Context UI that does not escape them. Shall I open a issue with them?
Comment #9
cpliakas CreditAttribution: cpliakas commentedThe /admin/structure/block/manage/facetapi/search_api@flatstore:block:size/configure in the URL is normal and works fine. However after looking into the core Drupal block list as you mentioned, the values are not properly escaped for classes and IDs. This is definitely an issue. I would actually say this is a core Drupal issue for not being able to handle this, but I also think that Facet API has to modify its strategy to deal with this. A couple of ideas:
Thanks for testing,
Chris
Comment #10
Anonymous (not verified) CreditAttribution: Anonymous commentedAha, well for short term we can just hash everything? It has no negative side-effect, I think. And for long term we can file an issue with D8 core. Maybe it will be ported back to D7.
Comment #11
cpliakas CreditAttribution: cpliakas commentedYeah, it's ugly but I think it might be the best option here. There is still a nice CSS ID for the unordered list inside of the block, so you shouldn't have to do much with the hash anyways.
Thanks for the suggestion,
Chris
Comment #12
Nick_vhMaybe if the problem is context you could take a look at http://drupal.org/project/context_block_class but this is only for D6 unfortunately...
Other then that I think hashing everything makes up for a solid solution indeed so my vote for that. I'm not sure why sanitizing is itchy? Maybe I have a wrong take on that but something like
$new_string = preg_replace(“/[^a-zA-Z0-9]/”, “”, $string);
Could be simple yet effective?
Comment #13
cpliakas CreditAttribution: cpliakas commentedWell, the way that sanitizing would have to be done is only for that specific page, and we wouldn't want to do it on input for a couple of reasons. First, characters such as "@" and ":" are part of the uniqueness of the name, so stripping them has a small chance of causing collisions. Probably not an issue, but why introduce the possibility of something like that. Second, the deltas contain colons which separate the different parts of the item. For example, "apachesolr@solr:block:bundle" means that the searcher is "apachesolr@solr", "block" is the realm, and "bundle" is the facet. Sanitizing this on input would be problematic because this system would be broken. Sanitizing this on output would have to be targeted to the pages that don't work correctly. I don't want to go down that road either.
So I guess hashing everything is the way to go here. We will need an update function to update the deltas in the block table as well.
Comment #14
cpliakas CreditAttribution: cpliakas commentedChanging title because the deltas are valid, they just contain characters that are problematic for JS.
Comment #15
cpliakas CreditAttribution: cpliakas commentedThe attached patch is my first attempt at fixing the issue. Make sure the blocks that are currently enabled remain enabled. Would love to get some testing before committing.
Thanks,
Chris
Comment #16
Anonymous (not verified) CreditAttribution: Anonymous commentedHi, I applied the patch and ran update.php without problems. Looks alright to me.
Comment #17
drunken monkeyWorks fine for me, too.
Comment #18
cpliakas CreditAttribution: cpliakas commentedTwo testers, and good ones at that. Works for me! Committed at 89cc84b.
Thanks for testing,
Chris
Comment #19.0
(not verified) CreditAttribution: commentedAdded summary up to issue #13.